Skip to content
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.

Sign up for GitHub

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

Jump to bottom

aiohttp not honoring no_proxy #4431

Closed
scirelli opened this issue Dec 9, 2019 · 10 comments
Closed

aiohttp not honoring no_proxy #4431

scirelli opened this issue Dec 9, 2019 · 10 comments
Assignees
@scirelli
Labels
enhancement

Comments

@scirelli
Copy link
Contributor

scirelli commented Dec 9, 2019 •
edited
Loading

Long story short

aiohttp not honoring no_proxy environment variable when trust_env is set.

Expected behaviour

When there is a no_proxy NO_PROXY environment variable set, URLs listed in that variable should not use http{s}_proxy

Actual behaviour

When trust_env is set and there are http_proxy and no_proxy set, the proxy is always used.

Steps to reproduce

Set http{s}_proxy and no_proxy, make a request to a domain listed in no_proxy. The request will still go through the proxy.

Your environment

Linux
aiohttp 3.6.2
Company proxy server
@asvetlov
Copy link
Member

asvetlov commented Dec 10, 2019 •
edited
Loading

The pull request is welcome!

@scirelli
Copy link
Contributor Author

Could you point me in the right direction?

I'm guessing this is where I'll need to check the no proxy list https://github.com/aio-libs/aiohttp/blob/master/aiohttp/client.py#L424 and blank the proxy if found?

@asvetlov
Copy link
Member

Hmm, interesting.
The real code for extracting the proxy list from env vars is proxies_from_env() function.

It doesn't parse itself but reuses getproxies() from urllib.request.
I did not investigate more.
Does getproxies() ignore NO_PROXY?

@scirelli
Copy link
Contributor Author

scirelli commented Dec 11, 2019 •
edited
Loading

No. That is why I suggesting doing it manually. I could only find a reference to no_proxy when urllib.request.ProxyHandler is used. I'm searching for something to compare the hostnames to the no_proxy list.

I could add a is_in_no_proxy_list(url) to the helpers.py module.

scirelli pushed a commit to scirelli/aiohttp that referenced this issue Dec 11, 2019
@scirelli-c1
Attempting to fix issue [aio-libs#4431](aio-libs#4431)
5d92cb5
@scirelli
Copy link
Contributor Author

Maybe a solution like 5d92cb5 might work. Would still have to write the tests. I should probably read up on the no_proxy list standards to make sure hostnames put in the no_proxy list are matching correctly.

@scirelli
Copy link
Contributor Author

Actually I looks like urllib has code to handle no_proxy. I'll use the appropriate function from there.

scirelli pushed a commit to scirelli/aiohttp that referenced this issue Dec 15, 2019
@scirelli-c1
Added no proxy check for issue aio-libs#4431
01303f4
@scirelli scirelli mentioned this issue Dec 15, 2019
Merged
5 tasks
@ghost
Copy link

ghost commented Feb 1, 2021

Any ETA for this? We're suffering from the same problem with aiohttp not honoring no_proxy vars.

@lbg-raghu-vennam
Copy link

Even I am interested in this fix as a third party application we are using is dependent on this library.

@lbg-raghu-vennam lbg-raghu-vennam mentioned this issue Feb 2, 2021
Closed
@paulbastian
Copy link

We tested these changes and they worked for us!

@scattym
Copy link

scattym commented Feb 20, 2021

Glad I found this bug report! Just wondering if there are any workarounds until this patch gets accepted?

I need to use a proxy to get to external services but that proxy has no access to other internal services, hence my need for no_proxy.

At the moment the only option I seem to have is to create separate sessions with and without proxies set, but that means my app would need to be aware of the network setup of where it gets installed, which is certainly not ideal. Unless there are other options I am missing?

scirelli added a commit to scirelli/aiohttp that referenced this issue Mar 23, 2021
@scirelli
Add support for the no_proxy env var mechanism in the HTTP client
86515c8 
PR aio-libs#4445 by @scirelli.
Fixes aio-libs#4431.

Co-authored-by: Steve Cirelli <[email protected]>
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
(cherry picked from commit 1a4126a)
@scirelli scirelli mentioned this issue Mar 23, 2021
Merged
5 tasks
scirelli added a commit to scirelli/aiohttp that referenced this issue Mar 23, 2021
@scirelli
Add support for the no_proxy env var mechanism in the HTTP client
8bc189b 
PR aio-libs#4445 by @scirelli.
Fixes aio-libs#4431.

Co-authored-by: Steve Cirelli <[email protected]>
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
(cherry picked from commit 1a4126a)
webknjaz pushed a commit that referenced this issue Apr 14, 2021
@scirelli
Add support for the no_proxy env var mechanism in the HTTP client (#…
d612748 
…5556)

PR #4445 by @scirelli.
Fixes #4431.

Co-authored-by: Steve Cirelli <[email protected]>
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
(cherry picked from commit 1a4126a)
commonism pushed a commit to commonism/aiohttp that referenced this issue Apr 27, 2021
@scirelli @scirelli-c1
@webknjaz @commonism
Add support for the no_proxy env var mechanism in the HTTP client
862bf86 
PR aio-libs#4445 by @scirelli.
Fixes aio-libs#4431.

Co-authored-by: Steve Cirelli <[email protected]>
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
commonism pushed a commit to commonism/aiohttp that referenced this issue Apr 27, 2021
@scirelli @scirelli-c1
@webknjaz @commonism
Add support for the no_proxy env var mechanism in the HTTP client
076ad6e 
PR aio-libs#4445 by @scirelli.
Fixes aio-libs#4431.

Co-authored-by: Steve Cirelli <[email protected]>
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
@priyaananthasankar priyaananthasankar mentioned this issue Jun 16, 2021
Closed
@celloni celloni mentioned this issue Mar 13, 2023
Merged
5 tasks
Dreamsorcerer added a commit that referenced this issue Apr 16, 2023
@celloni @Dreamsorcerer
Update documentation: complete client docs for honoring proxy env vars (
1c49738 
#7235)

<!-- Thank you for your contribution! -->

## What do these changes do?

* Update proxy support docs to honor `no_proxy` env
* Complete `trust_env` parameter description to honor `wss_proxy`,
`ws_proxy` or `no_proxy` env

## Are there changes in behavior for the user?

No

## Related issue number

Update docs for: #4431

## Checklist

- [ ] I think the code is well written
- [ ] Unit tests for the changes exist
- [x] Documentation reflects the changes
- [ ] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt`
  * The format is &lt;Name&gt; &lt;Surname&gt;.
  * Please keep alphabetical order, the file is sorted by names.
- [ ] Add a new news fragment into the `CHANGES` folder
  * name it `<issue_id>.<type>` for example (588.bugfix)
* if you don't have an `issue_id` change it to the pr id after creating
the pr
  * ensure type is one of the following:
    * `.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.
* Make sure to use full sentences with correct case and punctuation, for
example: "Fix issue with non-ascii contents in doctest text files."

---------

Co-authored-by: Marcel Loop <[email protected]>
Co-authored-by: Sam Bull <[email protected]>
celloni added a commit to celloni/aiohttp that referenced this issue Apr 17, 2023
@celloni
Update documentation: complete client docs for honoring proxy env vars (
2e27924 
aio-libs#7235)

<!-- Thank you for your contribution! -->

* Update proxy support docs to honor `no_proxy` env
* Complete `trust_env` parameter description to honor `wss_proxy`,
`ws_proxy` or `no_proxy` env

No

Update docs for: aio-libs#4431

- [ ] I think the code is well written
- [ ] Unit tests for the changes exist
- [x] Documentation reflects the changes
- [ ] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt`
  * The format is &lt;Name&gt; &lt;Surname&gt;.
  * Please keep alphabetical order, the file is sorted by names.
- [ ] Add a new news fragment into the `CHANGES` folder
  * name it `<issue_id>.<type>` for example (588.bugfix)
* if you don't have an `issue_id` change it to the pr id after creating
the pr
  * ensure type is one of the following:
    * `.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.
* Make sure to use full sentences with correct case and punctuation, for
example: "Fix issue with non-ascii contents in doctest text files."

---------

Co-authored-by: Marcel Loop <[email protected]>
Co-authored-by: Sam Bull <[email protected]>
(cherry picked from commit 1c49738)
@celloni celloni mentioned this issue Apr 17, 2023
Merged
5 tasks
Dreamsorcerer pushed a commit that referenced this issue Apr 17, 2023
@celloni
Patchback/backports/3.9/1c4973881f9f49e65720b89b194cc1d327e5bd32/pr-7…
2b3db3a 
…235 (#7256)

**This is a backport of the PR
#7235 as merged into master (
1c49738 )

## What do these changes do?

* Update proxy support docs to honor `no_proxy` env
* Complete `trust_env` parameter description to honor `wss_proxy`,
`ws_proxy` or `no_proxy` env

## Are there changes in behavior for the user?

No

## Related issue number

Update docs for: #4431

## Checklist

- [ ] I think the code is well written
- [ ] Unit tests for the changes exist
- [x] Documentation reflects the changes
- [ ] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt`
  * The format is &lt;Name&gt; &lt;Surname&gt;.
  * Please keep alphabetical order, the file is sorted by names.
- [ ] Add a new news fragment into the `CHANGES` folder
  * name it `<issue_id>.<type>` for example (588.bugfix)
* if you don't have an `issue_id` change it to the pr id after creating
the pr
  * ensure type is one of the following:
    * `.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.
* Make sure to use full sentences with correct case and punctuation, for
example: "Fix issue with non-ascii contents in doctest text files."
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@scirelli scirelli

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@asvetlov @webknjaz @scirelli @scattym @paulbastian @lbg-raghu-vennam