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

Output isn't quoted when multipart form values include spaces #5

Closed
salty-horse opened this issue Apr 12, 2018 · 14 comments
Closed

Output isn't quoted when multipart form values include spaces #5

salty-horse opened this issue Apr 12, 2018 · 14 comments

Comments

@salty-horse
Copy link

salty-horse commented Apr 12, 2018

When a mutlipart form value contains spaces, h2c outputs --form FOO=1 2 3 4 instead of --form 'FOO=1 2 3 4'.

Since h2c's output is a valid shell command, one should be careful with other characters such as ; or ', which may require quoting or escaping.

@bagder
Copy link
Member

bagder commented Apr 12, 2018

Oh right. I think I'll go with always-quoting the string and add escaping for a bunch of sensitive letters. What do you think about that?

@salty-horse
Copy link
Author

I struggle with getting shells to understand me sometimes. Always quoting seems like a good idea.

@bagder
Copy link
Member

bagder commented Apr 12, 2018

That also makes letters such as semicolons not needing extra escaping.

@bagder bagder closed this as completed in 45287bf Apr 12, 2018
@salty-horse
Copy link
Author

Since the fix uses double quotes instead of single quotes, a shell will expand $ variables.

POST / HTTP/1.1
Host: localhost:8000
User-Agent: curl/7.47.0
Accept: */*
Content-Length: 143
Expect: 100-continue
Content-Type: multipart/form-data; boundary=------------------------fdbf5f7d1c2a96b2

--------------------------fdbf5f7d1c2a96b2
Content-Disposition: form-data; name="FOO"

$PATH
--------------------------fdbf5f7d1c2a96b2--

outputs:
curl --http1.1 --user-agent "curl/7.47.0" --form FOO="$PATH" https://localhost:8000/, which is bad.

@bagder
Copy link
Member

bagder commented Apr 12, 2018

Adding $ and \ to the regex now. Anything else?

@salty-horse
Copy link
Author

Windows will expand %PATH%. I think single-quoting is the only way to avoid that if the output command should be cross-platform.

@bagder
Copy link
Member

bagder commented Apr 12, 2018

The windows command prompt doesn't support single quotes!

@bagder
Copy link
Member

bagder commented Apr 12, 2018

So quote % as well?

@salty-horse
Copy link
Author

Impossible...? In Windows, % is quoted with %%, but in a POSIX shell, %% is treated as %%.

@bagder
Copy link
Member

bagder commented Apr 12, 2018

Hm, okey, so maybe ignore Windows for now and add an option to make the output "windows compatible" later.

@slavaatsig
Copy link

slavaatsig commented Apr 12, 2018

Seems like best effort option is to provide the output suitable for the host platform with the option to generate output for other systems.

Looks like String::ShellQuote is a good bicycle for POSIX platforms.

If external library dependencies is a no-no, then for POSIX shells easiest way seems to use singular quotes so this is the only character that you will have to take care.

Reference: https://stackoverflow.com/a/24869016/7598113

@bagder
Copy link
Member

bagder commented Apr 12, 2018

BUGS
Only Bourne shell quoting is supported

... so it won't help us a whole lot.

@slavaatsig
Copy link

For Microsoft® Windows™ platforms there is an MSDN blog-post about the wrong ways to do so.

TL;DR: See conclusions at the end of the post, it should enlighten on how to do it properly.

P.S.
It would be more helpful if Microsoft would posted about how to do it in a proper way instead.

bagder added a commit that referenced this issue Apr 12, 2018
@bagder
Copy link
Member

bagder commented Apr 12, 2018

still, that requires a "windows output mode" option to quote in that style instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants