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

Bug: foo[] parameters in URLs cause an error #2044

Closed
perlun opened this Issue Nov 2, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@perlun

perlun commented Nov 2, 2017

I did this

$ curl "http://localhost:8000/api/some-resource?PYNO[]=ZA2134&PYNO[]=ZA2323&PYNO[]=100004232&PYNO[]=ZA1213" (the real list of PYNO[] values is much longer, abbreviated example.)

I expected the following

To get the result of the URL. 😄

With the -g / --globoff parameter, it works as expected.

I know that the behavior we have now has been this way for years, but... I still consider it a bad default personally, since I happen to use URLs with "array parameters" from time to time. It's also something that is easily supported in web frameworks like PHP. Given the "principle of least surprise", I think it would make more sense to invert the flag so that --globon is opt-in, for the cases when you want to work with URL sequences and ranges.

But maybe I'm the oddball here? I don't know, if people really like it this way, we shouldn't change it. On any occurrence, this would be a candidate for 8.x rather than something that should be done right now because we shouldn't make changes like this on a minor version release to avoid confusing people.

curl/libcurl version

$ curl -V
curl 7.47.1 (x86_64-apple-darwin15.3.0) libcurl/7.47.1 SecureTransport zlib/1.2.11
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: IPv6 Largefile NTLM NTLM_WB SSL libz UnixSockets

operating system

macOS High Sierra.

@bagder bagder added the cmdline tool label Nov 3, 2017

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Nov 3, 2017

Member

I think we should rather let [] remain as-is even when globbing is enabled as a work-around. I don't think we benefit much from downright rejecting the example input.

  1. curl supports and works with RFC 3986 URLs, and in such URLs there cannot legally be any [ or ] letters present since they are reserved and should be URL encoded to get used. The trend is however for browsers to move away from RFC3986 and so more and more "new" things find their way into the URLs that originally weren't.
  2. curl has had globbing enabled by default for such a long time that there are now likely to be a very large amount of existing command lines and scripts using it, that changing the default is likely to cause grief to a large amount of users.
Member

bagder commented Nov 3, 2017

I think we should rather let [] remain as-is even when globbing is enabled as a work-around. I don't think we benefit much from downright rejecting the example input.

  1. curl supports and works with RFC 3986 URLs, and in such URLs there cannot legally be any [ or ] letters present since they are reserved and should be URL encoded to get used. The trend is however for browsers to move away from RFC3986 and so more and more "new" things find their way into the URLs that originally weren't.
  2. curl has had globbing enabled by default for such a long time that there are now likely to be a very large amount of existing command lines and scripts using it, that changing the default is likely to cause grief to a large amount of users.
@perlun

This comment has been minimized.

Show comment
Hide comment
@perlun

perlun Nov 3, 2017

I think we should rather let [] remain as-is even when globbing is enabled as a work-around.

Makes sense! If you'd point me in the right direction in the code base, I might even be able to submit a pull request for this. (No promises, but it's good nonetheless to document "where" and "how" that fix should be rightfully made.)

perlun commented Nov 3, 2017

I think we should rather let [] remain as-is even when globbing is enabled as a work-around.

Makes sense! If you'd point me in the right direction in the code base, I might even be able to submit a pull request for this. (No promises, but it's good nonetheless to document "where" and "how" that fix should be rightfully made.)

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Nov 3, 2017

Member

We have code present that skips []-uses for an IPv6-address, so I think putting the check for an empty [] next to it would make sense.

Member

bagder commented Nov 3, 2017

We have code present that skips []-uses for an IPv6-address, so I think putting the check for an empty [] next to it would make sense.

@perlun perlun changed the title from Consider making --globoff be the default to Bug: foo[] parameters in URLs cause an error Nov 3, 2017

perlun pushed a commit to perlun/curl that referenced this issue Nov 3, 2017

@perlun

This comment has been minimized.

Show comment
Hide comment
@perlun

perlun Nov 3, 2017

We have code present that skips []-uses for an IPv6-address, so I think putting the check for an empty [] next to it would make sense.

Makes sense indeed! I did an attempt (my very first cURL pull request 🎉) in #2046, let's continue the discussion there.

perlun commented Nov 3, 2017

We have code present that skips []-uses for an IPv6-address, so I think putting the check for an empty [] next to it would make sense.

Makes sense indeed! I did an attempt (my very first cURL pull request 🎉) in #2046, let's continue the discussion there.

bagder added a commit that referenced this issue Nov 3, 2017

@bagder bagder closed this in 90abb74 Nov 4, 2017

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.