tool: change some fopen failures from warnings to errors #11677
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -2544,7 +2549,12 @@ | |||
| } | |||
| else { | |||
| fname = nextarg; | |||
| file = fopen(nextarg, FOPEN_READTEXT); | |||
| file = fopen(fname, FOPEN_READTEXT); | |||
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression
|
I haven't looked at all the CI results but I can see there are at least two tests failing 268 344 both of which expect curl to be able to handle non existent files. I can remove those tests but I think it's pretty clear now it was someone's intention to allow this behavior. For example rather than create foo.txt and bar.txt beforehand if the open fails just treat it as empty --etag-compare foo.txt --etag-save foo.txt --cookie bar.txt --cookie-jar bar.txt hm |
|
I plan to dial this PR back somewhat since it seems there are some intended cases where it may be useful to allow non-existent files to be treated as empty. |
- Error on missing input file for --data, --data-binary, --data-urlencode, --header, --variable, --write-out. Prior to this change if a user of the curl tool specified an input file for one of the above options and that file could not be opened then it would be treated as zero length data instead of an error. For example, a POST using `--data @filenametypo` would cause a zero length POST which is probably not what the user intended. Closes #xxxx
|
Ok I limited this change to error on missing input files for --data, --data-binary, --data-urlencode, --header, --variable, --write-out. test268 is going to fail because it tests to make sure --data will POST zero length content when passed a non-existent filename. IMO if the user passes a non-existent file for POST that's an accident not intentional. From 2005 639857c @bagder |
|
I think trying to post a file that can't be read should rather cause an error. |
test 268 is a check to make sure a zero-length POST is sent when passed a non-existent file but we now error in this case.
|
It does not make work easier when we merge changes that break CI. Just saying. |
Yes. This occasionally happens with all of us due to CI fatigue. We get the occasional random break due to CI arbitrary weirdness pretty much all the time and it makes the actual failures less likely to be noticed. I don't wait for 100% green check marks, instead I go through the CI and try to determine if my change is responsible (and oops, it was and I missed it). A lot of work has been done to improve the situation but it may happen occasionally. |
- Error on missing input file for --data, --data-binary, --data-urlencode, --header, --variable, --write-out. Prior to this change if a user of the curl tool specified an input file for one of the above options and that file could not be opened then it would be treated as zero length data instead of an error. For example, a POST using `--data @filenametypo` would cause a zero length POST which is probably not what the user intended. Closes curl#11677
Error if a user-specified file cannot be opened.
Change file2memory and file2string functions to return an error if passed a null FILE, instead of returning zero length data.
Prior to this change, if a user of the curl tool specified a file that could not be opened for reading data then curl would show a warning instead of an error and treat it as zero length data. For example, a POST using
--data @filenametypowould cause a zero length POST which is probably not what the user intended.Closes #xxxx