Skip to content

Comments

Fix HTTP/2 session receive window handling for small sizes#9117

Merged
bneradt merged 1 commit intoapache:masterfrom
bneradt:handle_small_h2_session_window_sizes
Oct 4, 2022
Merged

Fix HTTP/2 session receive window handling for small sizes#9117
bneradt merged 1 commit intoapache:masterfrom
bneradt:handle_small_h2_session_window_sizes

Conversation

@bneradt
Copy link
Contributor

@bneradt bneradt commented Oct 1, 2022

This fixes the way we handle session window sizes if the user configures proxy.config.http2.initial_window_size_in to be smaller than the HTTP/2 protocol's default value of 65,535. For streams, we can communicate to the client a smaller stream size via a SETTINGS frame, but not so for smaller session window sizes. The way HTTP/2 hosts are expected to reduce their receive session window size is by receiving DATA frames without replying with WINDOW_UPDATE frames until the window falls to the desired value. ATS already handled this latter behavior correctly (letting the window size fall without sending WINDOW_UPDATE frames for the session until it reached the desired smaller value). However, it incorrectly reported an error and sent a GOAWAY frame when the user sent DATA content above the reduced window size before ATS gave the client a chance to reduce it via DATA frames. This fixes the receive session window check to allow for a shrinking session window.

Fixes #9115

@bneradt bneradt added the HTTP/2 label Oct 1, 2022
@bneradt bneradt added this to the 10.0.0 milestone Oct 1, 2022
@bneradt bneradt requested a review from maskit October 1, 2022 21:37
@bneradt bneradt self-assigned this Oct 1, 2022
@bneradt bneradt force-pushed the handle_small_h2_session_window_sizes branch from fc16ae8 to deebda6 Compare October 3, 2022 18:05
This fixes the way we handle session window sizes if the user configures
proxy.config.http2.initial_window_size_in to be smaller than the HTTP/2
protocol's default value of 65,535. For streams, we can communicate to
the client a smaller stream size via a SETTINGS frame, but not so for
smaller session window sizes. The way HTTP/2 hosts are expected to
reduce their receive session window size is by receiving DATA frames
without replying with WINDOW_UPDATE frames until the window falls to the
desired value. ATS already handled this latter behavior correctly
(letting the window size fall without sending WINDOW_UPDATE frames for
the session until it reached the desired smaller value). However, it
incorrectly reported an error and sent a GOAWAY frame when the user sent
DATA content above the reduced window size before ATS gave the client a
chance to reduce it via DATA frames. This fixes the receive session
window check to allow for a shrinking session window.

Fixes apache#9115
@bneradt bneradt force-pushed the handle_small_h2_session_window_sizes branch from deebda6 to a99d18e Compare October 3, 2022 18:24
@maskit
Copy link
Member

maskit commented Oct 3, 2022

We should probably back port this to 9.2.x.

@bneradt bneradt merged commit 4fcb9b4 into apache:master Oct 4, 2022
@bneradt bneradt deleted the handle_small_h2_session_window_sizes branch October 4, 2022 14:30
@zwoop
Copy link
Contributor

zwoop commented Oct 4, 2022

@masaori335 @maskit This has some merge conflicts, possibly with something else you may have fixed which didn't go into 9.2.x? Can you take a look please?

maskit pushed a commit to maskit/trafficserver that referenced this pull request Oct 5, 2022
This fixes the way we handle session window sizes if the user configures
proxy.config.http2.initial_window_size_in to be smaller than the HTTP/2
protocol's default value of 65,535. For streams, we can communicate to
the client a smaller stream size via a SETTINGS frame, but not so for
smaller session window sizes. The way HTTP/2 hosts are expected to
reduce their receive session window size is by receiving DATA frames
without replying with WINDOW_UPDATE frames until the window falls to the
desired value. ATS already handled this latter behavior correctly
(letting the window size fall without sending WINDOW_UPDATE frames for
the session until it reached the desired smaller value). However, it
incorrectly reported an error and sent a GOAWAY frame when the user sent
DATA content above the reduced window size before ATS gave the client a
chance to reduce it via DATA frames. This fixes the receive session
window check to allow for a shrinking session window.

Fixes apache#9115

(cherry picked from commit 4fcb9b4)

 Conflicts:
	proxy/http2/Http2ConnectionState.cc
zwoop pushed a commit that referenced this pull request Oct 5, 2022
…9122)

This fixes the way we handle session window sizes if the user configures
proxy.config.http2.initial_window_size_in to be smaller than the HTTP/2
protocol's default value of 65,535. For streams, we can communicate to
the client a smaller stream size via a SETTINGS frame, but not so for
smaller session window sizes. The way HTTP/2 hosts are expected to
reduce their receive session window size is by receiving DATA frames
without replying with WINDOW_UPDATE frames until the window falls to the
desired value. ATS already handled this latter behavior correctly
(letting the window size fall without sending WINDOW_UPDATE frames for
the session until it reached the desired smaller value). However, it
incorrectly reported an error and sent a GOAWAY frame when the user sent
DATA content above the reduced window size before ATS gave the client a
chance to reduce it via DATA frames. This fixes the receive session
window check to allow for a shrinking session window.

Fixes #9115

(cherry picked from commit 4fcb9b4)

 Conflicts:
	proxy/http2/Http2ConnectionState.cc

Co-authored-by: Brian Neradt <brian.neradt@gmail.com>
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Feb 21, 2023
…) (apache#9122)

This fixes the way we handle session window sizes if the user configures
proxy.config.http2.initial_window_size_in to be smaller than the HTTP/2
protocol's default value of 65,535. For streams, we can communicate to
the client a smaller stream size via a SETTINGS frame, but not so for
smaller session window sizes. The way HTTP/2 hosts are expected to
reduce their receive session window size is by receiving DATA frames
without replying with WINDOW_UPDATE frames until the window falls to the
desired value. ATS already handled this latter behavior correctly
(letting the window size fall without sending WINDOW_UPDATE frames for
the session until it reached the desired smaller value). However, it
incorrectly reported an error and sent a GOAWAY frame when the user sent
DATA content above the reduced window size before ATS gave the client a
chance to reduce it via DATA frames. This fixes the receive session
window check to allow for a shrinking session window.

Fixes apache#9115

(cherry picked from commit 4fcb9b4)

 Conflicts:
	proxy/http2/Http2ConnectionState.cc

Co-authored-by: Brian Neradt <brian.neradt@gmail.com>
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Feb 21, 2023
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Feb 21, 2023
* asf/9.2.x:
  Updated ChangeLog
  Fail sni.yaml loading if related resources fail to load (apache#9132)
  fix contradicting documentation and say a bit about the resident size of a volume directory (apache#9133)
  AuTest automatic keylog file configuration (apache#9137)
  Traffic Dump: fix YAML format for CONNECT requests (apache#9139)
  Updated ChangeLog
  Remove intermediate buffer in PluginVC (apache#8698)
  Fix HTTP/2 session receive window handling for small sizes (apache#9117) (apache#9122)
  9.2: Fix s3_auth_config test output check (apache#9123)

 Conflicts:
	CHANGELOG-9.2.0
JosiahWI pushed a commit to JosiahWI/trafficserver that referenced this pull request Jul 19, 2023
This fixes the way we handle session window sizes if the user configures
proxy.config.http2.initial_window_size_in to be smaller than the HTTP/2
protocol's default value of 65,535. For streams, we can communicate to
the client a smaller stream size via a SETTINGS frame, but not so for
smaller session window sizes. The way HTTP/2 hosts are expected to
reduce their receive session window size is by receiving DATA frames
without replying with WINDOW_UPDATE frames until the window falls to the
desired value. ATS already handled this latter behavior correctly
(letting the window size fall without sending WINDOW_UPDATE frames for
the session until it reached the desired smaller value). However, it
incorrectly reported an error and sent a GOAWAY frame when the user sent
DATA content above the reduced window size before ATS gave the client a
chance to reduce it via DATA frames. This fixes the receive session
window check to allow for a shrinking session window.

Fixes apache#9115

(cherry picked from commit 4fcb9b4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improper HTTP/2 enforcement of a reduced session window size

3 participants