-
-
Notifications
You must be signed in to change notification settings - Fork 29.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-69152: Add _proxy_response_headers attribute to HTTPConnection #26152
Changes from 5 commits
0200725
7d8cefc
367893d
48d9119
ec7f577
6d59c02
232240e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -858,6 +858,7 @@ def __init__(self, host, port=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, | |
self._tunnel_host = None | ||
self._tunnel_port = None | ||
self._tunnel_headers = {} | ||
self._proxy_response_headers = None | ||
|
||
(self.host, self.port) = self._get_hostport(host, port) | ||
|
||
|
@@ -943,21 +944,15 @@ def _tunnel(self): | |
response = self.response_class(self.sock, method=self._method) | ||
(version, code, message) = response._read_status() | ||
|
||
self._proxy_response_headers = parse_headers(response.fp) | ||
|
||
if self.debuglevel > 0: | ||
for hdr, val in self._proxy_response_headers.items(): | ||
print("header:", hdr + ":", val) | ||
|
||
if code != http.HTTPStatus.OK: | ||
self.close() | ||
raise OSError(f"Tunnel connection failed: {code} {message.strip()}") | ||
while True: | ||
line = response.fp.readline(_MAXLINE + 1) | ||
if len(line) > _MAXLINE: | ||
raise LineTooLong("header line") | ||
if not line: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This condition There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can add the However, given that _tunnel method already prints headers in line 960 in The discussion in #69152 seems adding additional states to the _tunnel method, and adding debug response to those states. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @orsenthil, thanks for the review! I looked at the history, these lines were added when there was no equality check for the empty binary object below. That is, at some point they made sense, but after this commit, they ceased to be necessary. But we can add this check to the _read_headers method as a precaution. Do you think it should be done? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Maybe then it would be possible to start returning not OSError, but some custom error that can be catched in the library code (urllib3, requests) and automatically prepare data for authentication based on the information contained in _proxy_response_headers. Similar to how it is done in https://github.com/requests/requests-kerberos/pull/149/files. Maybe you know ways to solve these problems in a different way, I will be glad of any ideas and suggestions. |
||
# for sites which EOF without sending a trailer | ||
break | ||
if line in (b'\r\n', b'\n', b''): | ||
break | ||
|
||
if self.debuglevel > 0: | ||
print('header:', line.decode()) | ||
|
||
def connect(self): | ||
"""Connect to the host and port specified in __init__.""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Added attribute '_proxy_response_headers' to HTTPConnection class. This | ||
attribute contains the headers of the proxy server response to the CONNECT | ||
request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to how it is done in lines 337-341