FIX: BUG: IPv6 CIDR notation in NO_PROXY variable or option

[GenuaGSchulz]

The Bug

We noticed that using IPv6 CIDR-Notation in a NO_PROXY env var doesn't have the desired effect, contrary to what the documentation at https://everything.curl.dev/usingcurl/proxies/env.html explains.

How to reproduce:

• Install a local http proxy that denies everything and add its address to the HTTP_PROXY env var.
• Call curl -vv http://[::1] and observe that curl correctly tries to go through the proxy and fails to reach ::1.
• Now add a subnet to the NO_PROXY exception in CIDR notation that contains the host address ::1 by entering NO_PROXY="::1/64" curl -vv http://[::1]. We now expect to not access the proxy and reach ::1. We can observe that curl is still trying to go through the proxy, failing to reach ::1.

Debugging

I looked into the curl code and debugged a bit. An interesting find was that when using the above commands, the parameter name in Curl_check_noproxy() in lib/noproxy.c does not contain an address within brackets. Even if our target address is http://[::1], the value at name is "::1". This is unexpected, since Curl_check_noproxy() specificly checks name for brackets and only then interprets it as IPv6 address. The unit tests for that function have brackets around their IPv6 address strings, so they work.

The Fix

I didn't want to look further for how the brackets get removed in curl, because maybe there already is some other code now that relies on those brackets being removed. So my decision was to simply make Curl_check_noproxy() robust against that case and accept IPv6 with or without brackets. The fix is rather simple, as you can see.

By The Way

Non-CIDR IPv6 address strings (without /suffix) in the NO_PROXY env var just worked by accident, because for IPv6 addresses TYPE_HOST fell through in Curl_check_noproxy(), resulting in a simple string comparison between two addresses.

[bagder]

Nice catch!

How about extending the unit test 1614 as well with some Curl_check_noproxy tests passing in the host name as an IPv6 address without brackets?

[bagder]

Hm, in fact. Maybe we should just drop supporting the bracketed version completely as that's not how the name is ever passed in to the function outside of the unit tests? Then we could just drop the brackets from the host names in the unit test too.

[GenuaGSchulz]

I was thinking the same, but don't know the code as well to be sure that there is no case in which Curl_check_noproxy is called with a 'bracketed' IPv6 address string. So if you're telling me you're sure about this, I'll remove the path that checks for brackets and change the unit tests.

[bagder]

It is only called from a single spot in the source code so we know the brackets won't be there.

[GenuaGSchulz]

Done.

[bagder]

Thanks!

[GenuaGSchulz]

No, thank you for maintaining this important piece of open-source software!