Skip to content

Comments

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

Merged
zwoop merged 1 commit intoapache:9.2.xfrom
maskit:bp9117
Oct 5, 2022
Merged

Fix HTTP/2 session receive window handling for small sizes (#9117)#9122
zwoop merged 1 commit intoapache:9.2.xfrom
maskit:bp9117

Conversation

@maskit
Copy link
Member

@maskit maskit commented Oct 5, 2022

Back porting #9117 to 9.2.x. I had to make server_rwnd_is_shrinking public member to resolve conflicts.


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

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
@maskit maskit added HTTP/2 Backport Marked for backport for an LTS patch release labels Oct 5, 2022
@maskit maskit added this to the 9.2.0 milestone Oct 5, 2022
@maskit maskit requested review from bneradt and zwoop October 5, 2022 01:48
@maskit maskit self-assigned this Oct 5, 2022
@apache apache deleted a comment from maskit Oct 5, 2022
@zwoop
Copy link
Contributor

zwoop commented Oct 5, 2022

[approve ci autest]

1 similar comment
@ezelkow1
Copy link
Member

ezelkow1 commented Oct 5, 2022

[approve ci autest]

@bneradt
Copy link
Contributor

bneradt commented Oct 5, 2022

Looks like the s3_auth test needs to be updated in 9.2.x:

Generating Report: --------------
1 Tests were skipped:
--------------------------------------------------------------------------------
 Test "conn_timeout" Skipped
    File: conn_timeout.test.py
    Directory: /home/jenkins/workspace/Github_Builds/autest/src/tests/gold_tests/timeout
  Reason: Test requires privilege

 Test: s3_auth_config: Exception
    File: s3_auth_config.test.py
    Directory: /home/jenkins/workspace/Github_Builds/autest/src/tests/gold_tests/pluginTest/s3_auth
     Reason: Traceback (most recent call last):
       File "/home/jenkins/.local/share/virtualenvs/tests-iiJvYDvR/lib/python3.8/site-packages/autest/core/runtesttask.py", line 34, in __call__
         tl = self.__logic.Run(self.__test)
       File "/home/jenkins/.local/share/virtualenvs/tests-iiJvYDvR/lib/python3.8/site-packages/autest/runlogic/runlogic.py", line 18, in Run
         if not tmp.Start(obj):
       File "/home/jenkins/.local/share/virtualenvs/tests-iiJvYDvR/lib/python3.8/site-packages/autest/runlogic/test.py", line 90, in Start
         loadTest(self.__test)
       File "/home/jenkins/.local/share/virtualenvs/tests-iiJvYDvR/lib/python3.8/site-packages/autest/core/test.py", line 292, in loadTest
         execFile(fileName, locals, locals)
       File "/home/jenkins/.local/share/virtualenvs/tests-iiJvYDvR/lib/python3.8/site-packages/autest/common/execfile.py", line 16, in execFile
         exec(safeCompile(f.read(), fname), globals, locals)
       File "/home/jenkins/workspace/Github_Builds/autest/src/tests/gold_tests/pluginTest/s3_auth/s3_auth_config.test.py", line 65, in <module>
         ts.Disk.traffic_out.Content = "gold/s3_auth_parsing_ts.gold"
     AttributeError: 'Disk' object has no attribute 'traffic_out'

I can create a 9.2.x PR to fix that. Then hopefully the tests should work better.

@zwoop zwoop merged commit 89cb02f into apache:9.2.x Oct 5, 2022
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backport Marked for backport for an LTS patch release HTTP/2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants