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
-K/--config does not work correctly with the --next option #5120
Comments
If you have a proposed fix, please submit a PR with it! We should also create one or a few test cases based on your command lines above to make sure we can reproduce the problems and then that the future PR actually fixes them. |
By the way, this bug report falls out of trying to answer a question on StackOverflow. As an additional test, as indicated in my answer to that question, I had attempted to hoist the
|
Ensures that -K/--config inserts new items at the end of the list instead of overwriting the second item, and that after a -K/--config option has been parsed, the option parser's view of the current config is update. Fixes curl#5120
Summary
The handling of the linked list of
OperationConfig
structures is broken in bothparseconfig()
andparseargs()
, leading to a number of bugs.parseconfig()
(defined incurl/tool_parsecfg.c:74
) initializes(line 79):
struct OperationConfig *operation = global->first;
If the config file being parsed by
parseconfig
contains--next
, a newOperationConfig
will be installed:(line 244):
operation->next = malloc(sizeof(struct OperationConfig));
However,
operation
is not at the end of the linked list; it's at the beginning. Ifoperation->next
already exists (i.e. there were already at least two operations), it will be overwritten and it and the linked list of operations it points to will be lost (and the memory allocated to them will leak).To make this work,
operation
should have been initialised toglobal->last
.parse_args()
(defined insrc/tool_getparam.c:2237
) callsgetparameter()
at line 2260. Butgetparameter
might callparseconfig()
(if-K/--config
appears on the command line). Independently of issue (1),parseconfig()
may add a newOperationConfig
to the linked list, so whenparse_args()
returns, itsconfig
local variable no longer points at the currently activeOperationConfig
. (Indeed, this is also true inside ofgetparameter()
afterparseconfig()
returns, but I've convinced myself thatconfig
is not actually used after than point. Still, it maybe should be fixed.)Suggested fixes
Change
curl/tool_parsecfg.c:79
toAfter the call to
getparameter()
atsrc/tool_getparam.c:2260
, insertIt's possible that this also needs to be done after the call to
parseconfig()
atsrc/tool_getparam.c:1842
.As far as I can see, nothing stops a config file from including a recursive
-K/--config
option. To handle that possibility, insertafter the call to
getparameter()
atsrc/tool_parsecfg.c:235
.As an alternative to (2) and (3), consideration should be given to making the
config
argument ofgetparameter()
an "in-out" parameter by adding an extra level of indirection. In that case, it would make sense to add a similar "in-out" parameter toparseconfig
.Below, you'll find some tests.
I did this
First, I set up a simple HTTP server which echos back the request and any headers starting
T_
(to avoid too much output). Then I wrote the following simple bash function which generates a request in config-file format, starting with--next
. Note that since none of the option values includes whitespace, it's possible to also use the same script to produce command-line arguments:Invoking curl using command-line arguments works as expected:
But when the -K option is used, these fail in various ways.
If all of the request are in config files, then only the first and the last are processed. (The second and third are successively overwritten as described above.)
Other failure modes occur when config files are mixed with requests on the command line, because the OperationConfig being filled in by
parse_args
is not advanced when after the-K
option is processed. This results in the second command-line request overwriting preceding config file requests. (This is a similar symptom to the first problem, but the cause is different.)This can also result in options at the end of the command line being applied in surprising ways in certain circumstances, if not preceded on the command-line with
--next
:I expected the following
I expected all of the four-request command lines above to produce exactly the same output, and for the "surprise" headers to be added to the last config-file, not the first one. (Had the first request been a command-line request, these headers would have been added to it rather than the config-file request. That might be considered less surprising, but it's still surprising.)
curl/libcurl version
Tested with
and also with the latest tarball
But I think the behaviour dates back to this commit in early 2014: fc59a9e
operating system
Tested on
Linux rici-hp 4.15.0-32-generic #35-Ubuntu SMP Fri Aug 10 17:58:07 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
But it's probably cross-platform.
The text was updated successfully, but these errors were encountered: