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.

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

Handle space-separated values in NO_PROXY #5465

Closed
wants to merge 1 commit into from
Closed

Handle space-separated values in NO_PROXY #5465

wants to merge 1 commit into from

Conversation

michael-o
Copy link

Since there is no standard for the NO_PROXY environment variable allow values
to be comma- and space-separated values.

Since there is no standard for the NO_PROXY environment variable allow values
to be comma- and space-separated values.
@nateprewitt
Copy link
Member

Hi @michael-o, I did a bit of digging. While NO_PROXY has no formal spec, the majority of tools seem to be in agreement on its usage. I've been unable to find documentation for any common web tool that uses space-delimited lists for NO_PROXY. Curl, which we usually use as our model in situations like this, calls it out specifically as a "comma-separated list of hosts/domains".

Was there a reason you needed spaces introduced here?

@michael-o
Copy link
Author

michael-o commented Jul 21, 2020

@nateprewitt, yes flexibility in some degree: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=247947. Since curl works, I see no reason that py-requests shouldn't. I see no apparent harm by merging this.

@michael-o
Copy link
Author

@nateprewitt Do you see any change to have this merged? I will happily rebase if required.

@michael-o
Copy link
Author

Ping...

@sethmlarson
Copy link
Member

Curl documents as a comma separated list, I dont think we should do any different. Going to close this.

@sethmlarson sethmlarson closed this Jan 4, 2021
@michael-o
Copy link
Author

michael-o commented Jan 5, 2021

Well, a decision at least. Although my change is non-invasive. I would also concur your statement:

  1. Have a look at this: https://github.com/curl/curl/blob/648712eec1eedb05965b9b4d6dd457bda5d70481/lib/url.c#L2136-L2145
  2. This works for me with curl: NO_PROXY=localhost .siemens.net .siemens.com .siemens.de

As it seems the space is happily accepted:

osipovmi@deblndw011x:~
$ curl -I --verbose https://github.com
* Uses proxy env variable NO_PROXY == 'localhost .siemens.net .siemens.com .siemens.de'
* Uses proxy env variable HTTPS_PROXY == 'http://194.145.60.1:9400'
*   Trying 194.145.60.1:9400...
* Connected to 194.145.60.1 (194.145.60.1) port 9400 (#0)
* allocate connect buffer!
* Establish HTTP proxy tunnel to github.com:443
> CONNECT github.com:443 HTTP/1.1
> Host: github.com:443
> User-Agent: curl/7.74.0
> Proxy-Connection: Keep-Alive
>
< HTTP/1.1 200 Connection Established
HTTP/1.1 200 Connection Established
< Proxy-Agent: Zscaler/6.0
Proxy-Agent: Zscaler/6.0
<

* Proxy replied 200 to CONNECT request
* CONNECT phase completed!
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*  CAfile: none
*  CApath: /etc/ssl/certs/
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* CONNECT phase completed!
* CONNECT phase completed!
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
* TLSv1.3 (IN), TLS handshake, Certificate (11):
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
* TLSv1.3 (IN), TLS handshake, Finished (20):
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / TLS_AES_128_GCM_SHA256
* ALPN, server accepted to use http/1.1
* Server certificate:
*  subject: C=US; ST=California; L=San Francisco; O=GitHub, Inc.; CN=github.com
*  start date: May  5 00:00:00 2020 GMT
*  expire date: May 10 12:00:00 2022 GMT
*  subjectAltName: host "github.com" matched cert's "github.com"
*  issuer: C=US; O=DigiCert Inc; OU=www.digicert.com; CN=DigiCert SHA2 High Assurance Server CA
*  SSL certificate verify ok.
> HEAD / HTTP/1.1
> Host: github.com
> User-Agent: curl/7.74.0
> Accept: */*
>
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* old SSL session ID is stale, removing
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
HTTP/1.1 200 OK
< date: Tue, 05 Jan 2021 08:48:22 GMT
date: Tue, 05 Jan 2021 08:48:22 GMT
< content-type: text/html; charset=utf-8
content-type: text/html; charset=utf-8
< server: GitHub.com
server: GitHub.com
< status: 200 OK
status: 200 OK

Is that sufficient to change your mind?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants