Skip to content

Stop parsing completed HTTP proxy responses. - #1762

Closed
Sakshamm-Goyal wants to merge 1 commit into
python-websockets:mainfrom
Sakshamm-Goyal:agent/fix-http-proxy-terminal-lifecycle
Closed

Sakshamm-Goyal wants to merge 1 commit into
python-websockets:mainfrom
Sakshamm-Goyal:agent/fix-http-proxy-terminal-lifecycle

Conversation

@Sakshamm-Goyal

Copy link
Copy Markdown

Stop parsing completed HTTP proxy responses

When an HTTP proxy rejects CONNECT with 407 and closes the connection, connect() raises InvalidProxyStatus as expected, but the asyncio callback also raises AttributeError by resuming the completed response parser. EOF followed by connection loss can also feed EOF twice.

Treat the completed response future as terminal and share EOF handling. Seven tests cover success, rejection, malformed responses, cancellation, incomplete responses and trailing data. A local authenticated CONNECT tunnel reproduces the callback error before the fix and verifies echo/rejection without callback errors afterward.

Python 3.13 tests, full and per-module 100% coverage, Ruff and strict mypy pass. An initial synchronous reconnect timing failure passed on rerun with the repository CI timeout factor. Other Python/platform combinations weren't run locally.

Prepared and validated with an AI coding assistant. This is separate from the credential-decoding behavior in #1761.

@aaugustin

Copy link
Copy Markdown
Member

What problem is this actually causing in real life?

@aaugustin

Copy link
Copy Markdown
Member

the asyncio callback also raises AttributeError

What is "the asyncio callback" in this context?

@Sakshamm-Goyal

Copy link
Copy Markdown
Author

The concrete effect I reproduced is an extra, uncaught event-loop exception during cleanup after a proxy rejects CONNECT with 407 (for example, incorrect proxy credentials). connect() still raises the correct InvalidProxyStatus; this fix doesn't make the rejected connection succeed. I don't have a production incident or evidence of data loss or a hang to claim.

By “the asyncio callback” I meant _SelectorSocketTransport._call_connection_lost(), which calls HTTPProxyConnection.connection_lost(). The traceback is:

_SelectorSocketTransport._call_connection_lost()
  -> HTTPProxyConnection.connection_lost()
  -> HTTPProxyConnection.run_parser()
  -> response.status_code
AttributeError: 'NoneType' object has no attribute 'status_code'

The CONNECT response parser already completed when it reported 407. Cleanup resumes that exhausted generator, whose next StopIteration has no response value.

I re-ran the loopback integration on Python 3.13.11: a real local CONNECT proxy, a WebSocket echo server, and the public connect(..., proxy=...) API. Correct credentials echo successfully; incorrect credentials raise InvalidProxyStatus on both versions. With the pre-fix client at acff86b, the loop's exception handler additionally receives the AttributeError above. With this PR's head, it doesn't. So the demonstrated user-facing problem is a spurious asyncio error log / exception-handler invocation on an otherwise correctly handled proxy rejection, not a failed error return from connect().

@aaugustin

Copy link
Copy Markdown
Member

The actual problem is this:

$ WEBSOCKETS_DEBUG=1 python -m unittest -k test_authenticated_http_proxy_error
2026-10-03 14:15:03,385 DEBUG asyncio Using selector: KqueueSelector
2026-10-03 14:15:03,387 INFO mitmproxy.proxy.mode_servers HTTP(S) proxy listening at *:58080.
2026-10-03 14:15:03,459 DEBUG asyncio Using selector: KqueueSelector
2026-10-03 14:15:03,462 DEBUG asyncio Get address info localhost:58080, type=<SocketKind.SOCK_STREAM: 1>
2026-10-03 14:15:03,463 DEBUG asyncio Getting address info localhost:58080, type=<SocketKind.SOCK_STREAM: 1> took 1.458ms: [(<AddressFamily.AF_INET6: 30>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('::1', 58080, 0, 0)), (<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('127.0.0.1', 58080))]
2026-10-03 14:15:03,468 INFO mitmproxy.proxy.server client connect
2026-10-03 14:15:03,468 DEBUG asyncio <asyncio.TransportSocket fd=12, family=30, type=1, proto=6, laddr=('::1', 58297, 0, 0), raddr=('::1', 58080, 0, 0)> connected to localhost:58080: (<_SelectorSocketTransport fd=12 read=polling write=<idle, bufsize=0>>, <websockets.asyncio.client.HTTPProxyConnection object at 0x10adfa3c0>)
2026-10-03 14:15:03,471 ERROR asyncio Exception in callback _SelectorSocketTransport._call_connection_lost(None)
handle: <Handle _SelectorSocketTransport._call_connection_lost(None) created at /Users/myk/.pyenv/versions/3.14.4/lib/python3.14/asyncio/selector_events.py:902>
source_traceback: Object created at (most recent call last):
  File "/Users/myk/.pyenv/versions/3.14.4/lib/python3.14/unittest/mock.py", line 1917, in _inner
    return await f(*args, **kw)
  File "/Users/myk/dev/websockets/tests/asyncio/test_client.py", line 784, in test_authenticated_http_proxy_error
    async with connect("ws://example.com/"):
  File "/Users/myk/dev/websockets/src/websockets/asyncio/client.py", line 576, in __aenter__
    self.connection = await self
  File "/Users/myk/dev/websockets/src/websockets/asyncio/client.py", line 524, in connect
    connection = await self.open_tcp_connection()
  File "/Users/myk/dev/websockets/src/websockets/asyncio/client.py", line 415, in open_tcp_connection
    transport = await connect_http_proxy(
  File "/Users/myk/dev/websockets/src/websockets/asyncio/client.py", line 791, in connect_http_proxy
    transport.abort()
  File "/Users/myk/.pyenv/versions/3.14.4/lib/python3.14/asyncio/selector_events.py", line 830, in abort
    self._force_close(None)
  File "/Users/myk/.pyenv/versions/3.14.4/lib/python3.14/asyncio/selector_events.py", line 902, in _force_close
    self._loop.call_soon(self._call_connection_lost, exc)
Traceback (most recent call last):
  File "/Users/myk/dev/websockets/src/websockets/asyncio/client.py", line 738, in run_parser
    output = next(self.parser)
StopIteration

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/myk/.pyenv/versions/3.14.4/lib/python3.14/asyncio/events.py", line 94, in _run
    self._context.run(self._callback, *self._args)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/myk/.pyenv/versions/3.14.4/lib/python3.14/asyncio/selector_events.py", line 1196, in _call_connection_lost
    super()._call_connection_lost(exc)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^
  File "/Users/myk/.pyenv/versions/3.14.4/lib/python3.14/asyncio/selector_events.py", line 907, in _call_connection_lost
    self._protocol.connection_lost(exc)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^
  File "/Users/myk/dev/websockets/src/websockets/asyncio/client.py", line 770, in connection_lost
    self.run_parser()
    ~~~~~~~~~~~~~~~^^
  File "/Users/myk/dev/websockets/src/websockets/asyncio/client.py", line 742, in run_parser
    if 200 <= response.status_code < 300:
              ^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'status_code'
2026-10-03 14:15:03,471 INFO mitmproxy.proxy.server client disconnect

There's a spurious trackback because run_parser doesn't handle properly the case when it's called again after reading a response, typically when connection_lost is called after eof_received.

Since run_parser always calls self.response.set_result or self.response.set_exception when parsing completes, the easiest solution is to short-circuit when self.response.done().

@aaugustin aaugustin closed this in 32504a8 Oct 3, 2026
@aaugustin

Copy link
Copy Markdown
Member

My fix is functionally equivalent to the PR, but I'm making the change in only one place, and I think it's more logical there. Also it avoids unnecessary changes. I'm not sure what was up with all the tests, and I don't think this requires tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants