Add response proxy headers to ClientResponse #7152
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7152 +/- ##
==========================================
- Coverage 97.37% 97.34% -0.04%
==========================================
Files 106 106
Lines 31093 31138 +45
Branches 3875 3880 +5
==========================================
+ Hits 30278 30312 +34
- Misses 613 624 +11
Partials 202 202
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
psf-chronographer
bot
added
the
bot:chronographer:provided
|
I'm unsure about:
|
galaxyfeeder
force-pushed
the
response_proxy_headers
branch
from
98f1d13
to
5fe5cd0
Compare
galaxyfeeder
force-pushed
the
response_proxy_headers
branch
2 times, most recently
from
a352299
to
cc56d61
Compare
galaxyfeeder
force-pushed
the
response_proxy_headers
branch
from
cc56d61
to
6b7cdf4
Compare
|
Hello, @galaxyfeeder this problem is still relevant? |
| proxy.return_value = {"status": 200, "body": b"1" * (2**20)} | ||
| url = "https://www.google.com.ua/search?q=aiohttp proxy" | ||
|
|
||
| resp = await sess.get(url, proxy=proxy.url) |
There was a problem hiding this comment.
I think it's better to use the async with context manager because it ensures that resources will be properly closed after the async with block completes, even if an exception occurs. The code will also become more readable, because it does not require explicit invocation of closing methods (close(), release(), etc.).
Here's what the test implementation might look like:
async def test_proxy_https_proxy_headers(make_proxy_test_server: Any) -> None:
async with aiohttp.ClientSession() as session:
async with make_proxy_test_server() as proxy:
proxy.return_value = {"status": 200, "body": b"1" * (2 ** 20)}
url = "https://www.google.com.ua/search?q=aiohttp proxy"
async with session.get(url, proxy=proxy.url) as resp:
assert resp.proxy_headers == {"proxy-agent": "TestProxy/1.0"}
assert resp.headers == {}
There was a problem hiding this comment.
Agreed (note you can make code suggestions with the first toolbar button, which can then just be comitted).
There was a problem hiding this comment.
Sure, thank you, I'll take it note
|
Hi @DavidRomanovizc, it is still relevant for us. Right now we are currently running a fork which includes this changes. Reading the proxy headers coming from underlying |
|
I'd like @webknjaz to glance over this first, as I believe he's more familiar with the proxy stuff. I only have some minor comments. |
| @@ -881,6 +882,21 @@ async def xtest_proxy_from_env_https_with_auth( | |||
| assert r2.headers["Proxy-Authorization"] == auth.encode() | |||
|
|
|||
|
|
|||
| @pytest.mark.xfail | |||
There was a problem hiding this comment.
What's with the xfail? Surely the test should be working?
| proxy.return_value = {"status": 200, "body": b"1" * (2**20)} | ||
| url = "https://www.google.com.ua/search?q=aiohttp proxy" | ||
|
|
||
| resp = await sess.get(url, proxy=proxy.url) |
There was a problem hiding this comment.
Agreed (note you can make code suggestions with the first toolbar button, which can then just be comitted).
| self._proxy_headers = protocol.proxy_headers if protocol is not None else None | ||
| self._raw_proxy_headers = ( | ||
| protocol.raw_proxy_headers if protocol is not None else None | ||
| ) |
There was a problem hiding this comment.
| self._proxy_headers = protocol.proxy_headers if protocol is not None else None | |
| self._raw_proxy_headers = ( | |
| protocol.raw_proxy_headers if protocol is not None else None | |
| ) | |
| if protocol is None: | |
| self._proxy_headers = None | |
| self._raw_proxy_headers = None | |
| else: | |
| self._proxy_headers = protocol.proxy_headers | |
| self._raw_proxy_headers = protocol.raw_proxy_headers |
|
@Dreamsorcerer I've missed your review in my notifications. I will review your comments as soon as I have some spare time and I will sync with master. |
What do these changes do?
Add to the ClientResponse the headers and raw headers of the underlying CONNECT call in a proxied HTTPS request.
The headers and raw headers from the response are added to the ResponseHandler and later when processing the request are added to the response.
Are there changes in behavior for the user?
No.
Related issue number
Closes #6078
Checklist
CONTRIBUTORS.txtCHANGESfolder<issue_id>.<type>for example (588.bugfix)issue_idchange it to the pr id after creating the pr.feature: Signifying a new feature..bugfix: Signifying a bug fix..doc: Signifying a documentation improvement..removal: Signifying a deprecation or removal of public API..misc: A ticket has been closed, but it is not of interest to users.