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

allow CURLOPT_INFILE to be reset #283

Closed
wants to merge 3 commits into from

Conversation

msmuenchen
Copy link

This allows CURLOPT_INFILE to be reset to defaults without resetting the rest of the CURL object.

Closes bug #64247 and #44866

@weltling
Copy link
Contributor

Could you add some tests please?

@msmuenchen
Copy link
Author

Should I just test if setting CURLOPT_INFILE to NULL fails?

@weltling
Copy link
Contributor

In #64247 you've posted a scenario description, that's what should be tested I think. There is also a repro code you gave under http://pastebin.com/XLJsZ9JH which can be converted to a phpt. The built-in cli server might be helpful for that, just take a look at sapi/cli/tests/php_cli_server*phpt

@msmuenchen
Copy link
Author

The problem is that one can only test if setting to NULL failed, but not what really happens in the "underworld" of interface.c, as there is no get-method available for the CURL options.

@weltling
Copy link
Contributor

That's true, you can only test whether the php code works as expected and php don't crash and valgrind is happy. If that all is given one can expect the backend c code is correct. The better phpt is, the more the expectation is fulfilled.

@msmuenchen
Copy link
Author

Okay will do so :)

@weltling
Copy link
Contributor

@msmuenchen ping )

@smalyshev
Copy link
Contributor

Looks like it's fixed by 54fee59

@php-pulls
Copy link

Comment on behalf of tyrael at php.net:

implemented in http://git.php.net/?p=php-src.git;a=commit;h=54fee59598082052395f4011931096babd6f4013

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

Successfully merging this pull request may close these issues.

4 participants