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

Jump to bottom

noproxy tail matching still has an issue #9842

Closed
sthen opened this issue Nov 2, 2022 · 8 comments
Closed

noproxy tail matching still has an issue #9842

sthen opened this issue Nov 2, 2022 · 8 comments
Assignees
@bagder
Labels
connecting & proxies not-a-bug This is not a bug in curl

Comments

@sthen
Copy link
Contributor

sthen commented Nov 2, 2022

Contrary to https://github.com/curl/curl/blob/master/tests/unit/unit1614.c#L88, traditional behaviour for no_proxy is for example.com to also match host.example.com.

I did this

$ curl -V
curl 7.86.0 (x86_64-unknown-openbsd7.2) libcurl/7.86.0 LibreSSL/3.6.0 zlib/1.2.13 nghttp2/1.50.0
Release-Date: 2022-10-26
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS HSTS HTTP2 HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL threadsafe UnixSockets

$ no_proxy=google.com https_proxy=http://127.0.0.1:9999/ curl -sI https://www.google.com/ | head -5
curl: (7) Failed to connect to 127.0.0.1 port 9999 after 0 ms: Couldn't connect to server

I expected the following

$ curl -V
curl 7.85.0 (x86_64-unknown-openbsd7.2) libcurl/7.85.0 LibreSSL/3.6.0 zlib/1.2.12 nghttp2/1.49.0
Release-Date: 2022-08-31
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS HSTS HTTP2 HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL threadsafe UnixSockets

$ no_proxy=google.com https_proxy=http://127.0.0.1:9999/ curl -sI https://www.google.com/ | head -5
HTTP/2 200 
content-type: text/html; charset=ISO-8859-1
p3p: CP="This is not a P3P policy! See g.co/p3phelp for more info."
date: Wed, 02 Nov 2022 03:13:12 GMT
server: gws

As also seen in at least lynx, w3m, OpenBSD ftp(1).

curl/libcurl version

[see above]

operating system

OpenBSD $somehostname 7.2 GENERIC.MP#819 amd64
@sthen
Copy link
Contributor Author

sthen commented Nov 2, 2022

(I should add, adding commits efc286b and b830f9b doesn't help)

@bagder
Copy link
Member

bagder commented Nov 2, 2022

That is however how the functionality has been documented for curl for a very long time. The CURLOPT_NOPROXY docs from 7.85.0 says:

If the name in the noproxy list has a leading period, it is a domain match against the provided host name. This way ".example.com" will switch off proxy use for both "www.example.com" as well as for "foo.example.com".

@bagder bagder added the not-a-bug This is not a bug in curl label Nov 2, 2022
@bagder
Copy link
Member

bagder commented Nov 2, 2022

Added to the docs in 2d5fa35 it seems.

@sthen
Copy link
Contributor Author

sthen commented Nov 2, 2022

The lines just above that say "For example, example.com would match example.com,
example.com:80, and www.example.com, but not www.notanexample.com or
example.com.othertld."

@bagder
Copy link
Member

bagder commented Nov 2, 2022

Hah, yes it does. So, total confusion...

@hawicz
Copy link

hawicz commented Nov 4, 2022

This is a major breaking change from version 7.85.0 and earlier and is causing a fair number of issues because existing no_proxy settings that have worked for years no longer do so. I would argue that the original issue was a documentation bug, and the pre-existing behavior should have been retained.
Also, with the new version the order of entries in no_proxy matters, no_proxy=example.com,.example.com works as expected, but no_proxy=.example.com,example.com incorrectly tries to use the proxy.

Can we please revert to the original behavior and update the man page instead?

@sthen
Copy link
Contributor Author

sthen commented Nov 4, 2022

FWIW, the reason I noticed this is because I have a couple of dozen machines which had curl in the pipeline for checking for updates to packages, and after updating to 7.86.0 for the IDN security fix they were no longer able to do so because they relied on no_proxy to reach the relevant server. I've since changed how they do things, but wanted to mention it as an example of one type of problem that can be caused by this.

If it is going to stay like this, then at least an explicit note in release notes would be appreciated.

@bagder bagder self-assigned this Nov 4, 2022
bagder added a commit that referenced this issue Nov 5, 2022
@bagder
noproxy: tailmatch like in 7.85.0 and earlier
5370caf 
A regfression in 7.86.0 (via 1e9a538) made the tailmatch work
differently than before. This restores the logic to how it used to work:

All names listed in NO_PROXY are tailmatched against the used domain
name, if it the lengths are identical a needs a full match.

Update the docs, update test 1614.

Reported-by: Stuart Henderson
Fixes #9842
@bagder bagder mentioned this issue Nov 5, 2022
Closed
@bagder
Copy link
Member

bagder commented Nov 5, 2022

I will appreciate if you help me verify if #9858 brings back the old logic proper.

bagder added a commit that referenced this issue Nov 6, 2022
@bagder
noproxy: tailmatch like in 7.85.0 and earlier
b1dbf4c 
A regfression in 7.86.0 (via 1e9a538) made the tailmatch work
differently than before. This restores the logic to how it used to work:

All names listed in NO_PROXY are tailmatched against the used domain
name, if the lengths are identical it needs a full match.

Update the docs, update test 1614.

Reported-by: Stuart Henderson
Fixes #9842
Closes #9858
@bagder bagder closed this as completed in b1953c1 Nov 7, 2022
@henning-schild henning-schild mentioned this issue Nov 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bagder bagder

Labels
connecting & proxies not-a-bug This is not a bug in curl
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

noproxy: tailmatch like in 7.85.0 and earlier curl/curl
3 participants
@bagder @hawicz @sthen