diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e8af71efb7..88ea28459a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,7 +45,7 @@ jobs: python-version: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13"] os: - macos-12 - - windows-2019 + - windows-2022 - ubuntu-20.04 # OpenSSL 1.1.1 - ubuntu-latest # OpenSSL 3.0 nox-session: [''] @@ -89,7 +89,7 @@ jobs: os: ubuntu-22.04 runs-on: ${{ matrix.os }} - name: ${{ fromJson('{"macos-12":"macOS","windows-2019":"Windows","ubuntu-latest":"Ubuntu","ubuntu-20.04":"Ubuntu 20.04 (OpenSSL 1.1.1)","ubuntu-latest":"Ubuntu Latest (OpenSSL 3+)"}')[matrix.os] }} ${{ matrix.python-version }} ${{ matrix.nox-session }} + name: ${{ fromJson('{"macos-12":"macOS","windows-2022":"Windows","ubuntu-latest":"Ubuntu","ubuntu-20.04":"Ubuntu 20.04 (OpenSSL 1.1.1)","ubuntu-latest":"Ubuntu Latest (OpenSSL 3+)"}')[matrix.os] }} ${{ matrix.python-version }} ${{ matrix.nox-session }} continue-on-error: ${{ matrix.experimental }} timeout-minutes: 40 steps: diff --git a/CHANGES.rst b/CHANGES.rst index 26ce19abcd..8b516419c6 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,10 @@ +2.8.907 (2024-08-20) +==================== + +- Fixed http2 maximum frame size error when the remote explicitly set a lower value than the default blocksize. + This can happen when facing old server like Apache < 2.5 see https://github.com/apache/httpd/commit/ff6b8026acb8610e4faf10ee345141a3da85946e + Now we monitor the max_frame setting value to ensure we don't exceed it. + 2.8.906 (2024-08-15) ==================== diff --git a/docker-compose.win.yaml b/docker-compose.win.yaml index ef4c12312e..769896604f 100644 --- a/docker-compose.win.yaml +++ b/docker-compose.win.yaml @@ -1,6 +1,6 @@ services: proxy: - image: traefik:v2.10.4-windowsservercore-1809 + image: traefik:v3.1-windowsservercore-ltsc2022 restart: unless-stopped healthcheck: test: [ "CMD", "traefik" ,"healthcheck", "--ping" ] @@ -51,7 +51,6 @@ services: - --entrypoints.alt-http.address=:9999 - --entrypoints.alt-https.address=:8754 # QUIC Related Configuration - - --experimental.http3=true - --entrypoints.https.http3=true - --entrypoints.alt-https.http3=false # Enable the access log, with HTTP requests diff --git a/docker-compose.yaml b/docker-compose.yaml index 715df6fa03..a66d5739a1 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -1,6 +1,6 @@ services: proxy: - image: traefik:v2.10.4 + image: traefik:v3.1 restart: unless-stopped healthcheck: test: [ "CMD", "traefik" ,"healthcheck", "--ping" ] @@ -48,7 +48,6 @@ services: - --entrypoints.alt-http.address=:9999 - --entrypoints.alt-https.address=:8754 # QUIC Related Configuration - - --experimental.http3=true - --entrypoints.https.http3=true - --entrypoints.alt-https.http3=false # Enable the access log, with HTTP requests diff --git a/src/urllib3/_async/connection.py b/src/urllib3/_async/connection.py index 760138bd14..4556d99d13 100644 --- a/src/urllib3/_async/connection.py +++ b/src/urllib3/_async/connection.py @@ -372,7 +372,7 @@ async def request( chunks_and_cl = body_to_chunks( body, method=method, - blocksize=self.blocksize, + blocksize=self.max_frame_size, force=self._svn != HttpVersion.h11, ) is_sending_string = chunks_and_cl.is_string diff --git a/src/urllib3/_async/connectionpool.py b/src/urllib3/_async/connectionpool.py index 51c8b35740..d5d4b85def 100644 --- a/src/urllib3/_async/connectionpool.py +++ b/src/urllib3/_async/connectionpool.py @@ -1087,6 +1087,10 @@ async def _make_request( ) conn.timeout = read_timeout + http_vsn_str = ( + conn._http_vsn_str + ) # keep vsn here, as conn may be upgraded afterward. + # Receive the response from the server try: response = await conn.getresponse(police_officer=self.pool) @@ -1106,7 +1110,7 @@ async def _make_request( method, url, # HTTP version - conn._http_vsn_str, + http_vsn_str, response.status, response.length_remaining, ) diff --git a/src/urllib3/_version.py b/src/urllib3/_version.py index 777c40c994..120b6a18b2 100644 --- a/src/urllib3/_version.py +++ b/src/urllib3/_version.py @@ -1,4 +1,4 @@ # This file is protected via CODEOWNERS from __future__ import annotations -__version__ = "2.8.906" +__version__ = "2.8.907" diff --git a/src/urllib3/backend/_async/hface.py b/src/urllib3/backend/_async/hface.py index e3dfaa2f74..b70285bb35 100644 --- a/src/urllib3/backend/_async/hface.py +++ b/src/urllib3/backend/_async/hface.py @@ -122,6 +122,23 @@ def is_saturated(self) -> bool: def is_multiplexed(self) -> bool: return self._protocol is not None and self._protocol.multiplexed + @property + def max_frame_size(self) -> int: + """ + The remote may require us to a lower blocksize + than defined. This property will avoid relying + too much on defined blocksize. + """ + if self._protocol is None: + return self.blocksize + + try: + remote_max_size = self._protocol.max_frame_size() + except NotImplementedError: + return self.blocksize + + return remote_max_size if self.blocksize > remote_max_size else self.blocksize + async def _new_conn(self) -> AsyncSocket | None: # type: ignore[override] # handle if set up, quic cache capability. thus avoiding first TCP request prior to upgrade. if ( diff --git a/src/urllib3/backend/_base.py b/src/urllib3/backend/_base.py index acabcdae9a..399f19d6b1 100644 --- a/src/urllib3/backend/_base.py +++ b/src/urllib3/backend/_base.py @@ -429,6 +429,10 @@ def is_idle(self) -> bool: def is_multiplexed(self) -> bool: raise NotImplementedError + @property + def max_frame_size(self) -> int: + raise NotImplementedError + def _upgrade(self) -> None: """Upgrade conn from svn ver to max supported.""" raise NotImplementedError diff --git a/src/urllib3/backend/hface.py b/src/urllib3/backend/hface.py index 0e5ba5ab1b..c7bd9880c3 100644 --- a/src/urllib3/backend/hface.py +++ b/src/urllib3/backend/hface.py @@ -136,6 +136,18 @@ def is_saturated(self) -> bool: def is_multiplexed(self) -> bool: return self._protocol is not None and self._protocol.multiplexed + @property + def max_frame_size(self) -> int: + if self._protocol is None: + return self.blocksize + + try: + remote_max_size = self._protocol.max_frame_size() + except NotImplementedError: + return self.blocksize + + return remote_max_size if self.blocksize > remote_max_size else self.blocksize + def _new_conn(self) -> socket.socket | None: # handle if set up, quic cache capability. thus avoiding first TCP request prior to upgrade. if ( diff --git a/src/urllib3/connection.py b/src/urllib3/connection.py index 018a3bb86f..b38e377b8e 100644 --- a/src/urllib3/connection.py +++ b/src/urllib3/connection.py @@ -379,7 +379,7 @@ def request( chunks_and_cl = body_to_chunks( body, method=method, - blocksize=self.blocksize, + blocksize=self.max_frame_size, force=self._svn != HttpVersion.h11, ) is_sending_string = chunks_and_cl.is_string diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py index 8017777690..a08ed3f702 100644 --- a/src/urllib3/connectionpool.py +++ b/src/urllib3/connectionpool.py @@ -1065,6 +1065,10 @@ def _make_request( conn.timeout = read_timeout # Receive the response from the server + http_vsn_str = ( + conn._http_vsn_str + ) # keep vsn here, as conn may be upgraded afterward. + try: response = conn.getresponse(police_officer=self.pool) except (BaseSSLError, OSError) as e: @@ -1083,7 +1087,7 @@ def _make_request( method, url, # HTTP version - conn._http_vsn_str, + http_vsn_str, response.status, response.length_remaining, ) diff --git a/src/urllib3/contrib/hface/protocols/_protocols.py b/src/urllib3/contrib/hface/protocols/_protocols.py index c7ecf37f2d..3850ab45b7 100644 --- a/src/urllib3/contrib/hface/protocols/_protocols.py +++ b/src/urllib3/contrib/hface/protocols/_protocols.py @@ -63,6 +63,12 @@ def should_wait_remote_flow_control( """ raise NotImplementedError + def max_frame_size(self) -> int: + """ + Determine if the remote set a limited size for each data frame. + """ + raise NotImplementedError + class OverTCPProtocol(BaseProtocol, metaclass=ABCMeta): """ diff --git a/src/urllib3/contrib/hface/protocols/http2/_h2.py b/src/urllib3/contrib/hface/protocols/http2/_h2.py index 33411969c3..3008a51a93 100644 --- a/src/urllib3/contrib/hface/protocols/http2/_h2.py +++ b/src/urllib3/contrib/hface/protocols/http2/_h2.py @@ -85,6 +85,10 @@ def __init__( self._terminated: bool = False self._goaway_to_honor: bool = False self._max_stream_count = self._connection.remote_settings.max_concurrent_streams + self._max_frame_size = self._connection.remote_settings.max_frame_size + + def max_frame_size(self) -> int: + return self._max_frame_size # type: ignore[no-any-return] @staticmethod def exceptions() -> tuple[type[BaseException], ...]: @@ -155,8 +159,10 @@ def _map_events(self, h2_events: list[jh2.events.Event]) -> Iterator[Event]: yield DataReceived(e.stream_id, e.data, end_stream=end_stream) elif isinstance(e, jh2.events.StreamReset): self._open_stream_count -= 1 - stream = self._connection.streams.pop(e.stream_id) - self._connection._closed_streams[e.stream_id] = stream.closed_by + # event StreamEnded may occur before StreamReset + if e.stream_id in self._connection.streams: + stream = self._connection.streams.pop(e.stream_id) + self._connection._closed_streams[e.stream_id] = stream.closed_by yield StreamResetReceived(e.stream_id, e.error_code) elif isinstance(e, jh2.events.ConnectionTerminated): # ConnectionTerminated from h2 means that GOAWAY was received. @@ -202,6 +208,7 @@ def bytes_received(self, data: bytes) -> None: self._max_stream_count = ( self._connection.remote_settings.max_concurrent_streams ) + self._max_frame_size = self._connection.remote_settings.max_frame_size def bytes_to_send(self) -> bytes: return self._connection.data_to_send() # type: ignore[no-any-return] diff --git a/traefik/patched.Dockerfile b/traefik/patched.Dockerfile index 0a2c851ac8..8263b87b73 100644 --- a/traefik/patched.Dockerfile +++ b/traefik/patched.Dockerfile @@ -7,7 +7,7 @@ COPY . . RUN mkdir dist RUN go build -ldflags="-s" -o dist/go-httpbin.exe ./cmd/go-httpbin -FROM mcr.microsoft.com/windows/nanoserver:ltsc2019 +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 COPY --from=build /go/src/github.com/mccutchen/go-httpbin/dist /app