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

Bugfix: Fix segfault when -H @headerfile is empty #2797

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@sm0svx
Contributor

sm0svx commented Jul 26, 2018

This commit will fix a bug where the curl binary would crash if the -H
command line option was given a filename to read using the @filename
syntax but that file was empty.

Bugfix: Fix segfault when -H @headerfile is empty
This commit will fix a bug where the curl binary would crash if the -H
command line option was given a filename to read using the @filename
syntax but that file was empty.
@bagder

This comment has been minimized.

Member

bagder commented Jul 27, 2018

Lovely. Any chance you could add a test case for this as well?

@bagder

bagder approved these changes Jul 27, 2018

@jay jay added the cmdline tool label Jul 27, 2018

@jay

This comment has been minimized.

Member

jay commented Jul 27, 2018

This looks like it was a simple oversight I don't think it needs a test case. Also I think a better fix for this would be return an empty buffer if the file is empty.

@sm0svx

This comment has been minimized.

Contributor

sm0svx commented Jul 27, 2018

@jay: I felt it was a lot safer to change the error handling locally in that spot only since the file2memory function is used in a number of other places. The risk of introducing new bugs is much bigger if the behaviour of the file2memory function is changed, don't you agree?

@bagder: I find it a bit hard to understand how the tests are supposed to work. I used cmake to build curl but a "make test" does not seem to be the right way to trigger the tests. I get:

Running tests...
Test project /tmp/curl/build
No tests were found!!!

Reading the README in the tests directory I get the impression that autotools should be used instead of cmake. Is autotools used when building tests and cmake when building the lib and binaries? I'm a bit confused. Can you point me in the right direction, or should we skip writing a test for this as @jay suggests?

@sm0svx

This comment has been minimized.

Contributor

sm0svx commented Jul 27, 2018

I now saw the note about the cmake buildsystem beeing poorly maintained. I'll try building it all using autotools instead. I guess that is the preferred way to build curl.

@bagder

This comment has been minimized.

Member

bagder commented Jul 28, 2018

I'll add a test case when I merge this!

@bagder bagder closed this in 3e9b3a3 Jul 28, 2018

bagder added a commit that referenced this pull request Jul 28, 2018

@bagder

This comment has been minimized.

Member

bagder commented Jul 28, 2018

Thanks!

xquery added a commit to xquery/curl that referenced this pull request Aug 9, 2018

curl: Fix segfault when -H @headerfile is empty
The curl binary would crash if the -H command line option was given a
filename to read using the @filename syntax but that file was empty.

Closes curl#2797

xquery added a commit to xquery/curl that referenced this pull request Aug 9, 2018

falconindy added a commit to falconindy/curl that referenced this pull request Sep 10, 2018

curl: Fix segfault when -H @headerfile is empty
The curl binary would crash if the -H command line option was given a
filename to read using the @filename syntax but that file was empty.

Closes curl#2797

falconindy added a commit to falconindy/curl that referenced this pull request Sep 10, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Oct 26, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.