-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
noproxy: explictly support space-separated no proxy hosts/domains #10209
Conversation
For a long time curl implicitly supported space-seprated, along with comma-separated, hosts/domains either via API or no_proxy/NO_PROXY. To clarify and extend compatiblity explicitly support/document the status quo now.
I checked, it was documented as a comma-separated list already before my recent refresh of the noproxy code. It was never documented or otherwise suggested that it was fine with just space separation. That was just accidentally working. I don't quite understand what the benefit is (for the world) to make the formerly undocumented feature officially supported, especially when other implementations don't support space either and even that gitlab page suggests that the common style should be comma-separated. I understand the benefit to you since doing as this PR would bring back a noproxy string you used before to work again. To me it seems to be less work to rather just insert commas in your list. |
OK. let me clarify my motivation for this. There are aspects which led me to this PR:
I see two options to resolve this properly:
Either way, a predictable, well understood behavior is important. Let me know what you think! |
Agreed. The lowest common denominator among implementations as far as separators go seems to be the comma. Not space. Making curl support spaces (again) will certainly bring back the previous functionality for users of curl and libcurl but it seems to little to unify the implementations; quite the opposite. It seems many of the implementations don't support space now and have rejected doing so. Opening up for space now will only bring back an uncertainty and ambiguity that doesn't help. To me, it seems to be generally more helpful to the "noproxy community" at large to not allow spaces, since using commas has a much greater chance for interop. This also matches what the gitlab page suggests a "standard" noproxy parser should do - and I believe curl actually works exactly like the bullet points say under "Standardizing" on that page. |
This would mean that you have to tighten the parser, no? This would be logical. |
I thought you wanted to bring back space-separation because it does not currently work. What needs to be tightened do you mean? |
You misunderstood. It does work, but is not official. You said that it should not be supported, this would mean that it would not be supported unofficially (code change). I, of course, prefer to make it official, but, of course, the decision is up to you. |
Sorry for being slow. So it does still work, and it has worked for a long time. That puts it in a slightly different light to me. But only slightly. It was never documented to work with spaces as separators and only a small subset of noproxy parsers seem to accept them as separators. The remaining question is then probably only if we should rather leave it undocumented and still insist on commas in the documentation, or if we should go ahead and make commas required between patterns. |
Correct, that is the cardinal question. Working example with spaces:
Through proxy:
|
How about this approach: We deprecate support for space-separated patterns in the noproxy string. By this I mean that we set a time in the future (at least 6 months into the future) when we declare we will remove support for them. Until then we produce a warning into the verbose output when a space-separated pattern is used. This gives everyone a chance to react and transition until we take it away. |
This is something I can live with. Willing to test a patch. Consider that for proxy users in corporate environments it may take longer to catch up. I'd expect that end of 2023 or so is reasonable. |
Ok, let's make it 18 months from now. July 2024. I'll make a PR you can try. |
Thanks, looking forward too. |
To be removed in July 2024. Assisted-by: Michael Osipov Fixes curl#10209 Closes curl#10215
For a long time curl implicitly supported space-separated, along with comma-separated, hosts/domains either via API or no_proxy/NO_PROXY. To clarify and extend flexibility explicitly support/document the status quo now.
For the past 10 years or so I have been using this on FreeBSD to access the outside world with curl and especially with Git.
Motivation:
py-requests
and stated that curl unofficially supports this too. Well, the maintainers said: not documented, closed and locked which is a pity: Handle space-separated values in NO_PROXY psf/requests#5465I have also put this into consideration.