-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
curl: allow --header and --proxy-header read from file #1486
Conversation
@bagder, thanks for your PR! By analyzing the history of the files in this pull request, we identified @yangtse, @captain-caveman2k and @kdudka to be potential reviewers. |
Oops, |
Ref: https://curl.haxx.se/mail/archive-2017-05/0029.html I don't know what he's talking about with the securing curlrc, if he uses --config foo it's still going to read the default curlrc. I will follow up on that on the list. So a change for this isn't necessary. Also this is breaking due to lack of strtok_r in Windows CI. strtok.h is only in the library so that would need to be changed or you could just use strtok and add a comment overidding checksrc and saying allow strtok since not threaded |
It isn't necessary for only that reason, no. But this is not the first time I've seen this request and I think it fits in how curl works already to have us consider supporting it. |
Thanks for the quick work. Sorry I missed that --config doesn't supersede the user's One comment on the code - you do an fopen with mode 'rb'. 'b' is binary (on systems that care). This is text input. So you probably want 'r'... you can't count on in the file for building the header; it might be there; might not; might be before the or after. Or you might get record length bytes... or untranslated wide chars. Thanks again. |
I don't see that as a safe bet at all. If you would save regular incoming received HTTP headers in a file, they are CRLF separated, and if you then use such a file as input to this on a *nix... I think it is safest to check for both. Updated version coming up... |
So many headers can be provided as @filename. Suggested-by: Timothe Litt
4951465
to
956c859
Compare
I'm not sure I agree. Why would I edited the wording for
We tried that in the past and it turns out really hard and more confusing than this. Most options are related to at least a few others so in which order should they come, really? |
Well, that's a question of where the bug is. Both the network and filesystems have defined rules, and I'd argue that whomever is saving headers to a text file should be removing the network-required CR,LF and writing to a text file with \n. That's portable. \n is defined to be "whatever the target filesystem wants"; CRLF; CR; LFCR; LF; or even (control-word, data). Headers are network ASCII. Of course, the file should have been opened with "w", not "wb". Then the C RTL won't add CR when it's written. Except on a CRLF file system. Opening that file with 'r' will always yield \n -- even on a CRLF system, where the C RTL will remove the CR. (And \n isn't always 012...) When you play by the rules, what's written will always be correct for native utilities - e.g. diff, your favorite editor, etc. You can be tolerant of stray on input from files - but definitely should not be reading and writing text files with 'b' and '\n'. You must of course, deal with network ASCII by it's rules - emit , be tolerant of bare , and use \012 for LF - not '\n'. (On some systems, \n is 015 - and odder things. ) In any case, I appreciate your work. It's rare to see such a quick response. |
Test 1147 that tests this is failing the torture tests: https://curl.haxx.se/dev/log.cgi?id=20170625054623-24701#prob1154 |
Bug: #1486 (comment) Reported-by: Dan Fandrich
Fixed in 922f800, thanks. |
So many headers can be provided as @filename.
Suggested-by: Timothe Litt