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

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

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

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

perlun opened this issue Nov 2, 2017 · 4 comments
Labels
cmdline tool

Comments

@perlun
Copy link

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
Copy link
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
Copy link
Author

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
Copy link
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 Consider making --globoff be the default Bug: foo[] parameters in URLs cause an error Nov 3, 2017
perlun added a commit to perlun/curl that referenced this issue Nov 3, 2017
@perlun
Bug fix: handle [] in URLs more gracefully
9c922f6 
Fixes curl#2044.
@perlun perlun mentioned this issue Nov 3, 2017
Closed
@perlun
Copy link
Author

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
curl: pass through [] in URLs instead of calling globbing error
b2111d0 
Assisted-by: Per Lundberg
Fixes #2044
Closes #2046
@bagder bagder mentioned this issue Nov 3, 2017
Closed
@bagder bagder closed this as completed 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.
Assignees
No one assigned
Labels
cmdline tool
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bagder @perlun