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

http: don't send 100-continue for short PUT requests #10740

Closed
wants to merge 3 commits into from

Conversation

dfandrich
Copy link
Contributor

This is already how curl is documented to behave in Everything curl, but in
actuality only short POSTs skip this. This should knock 30 seconds off a
full run of the test suite since the 100-continue timeout will no longer be
hit.

This is already how curl is documented to behave in Everything curl, but
in actuality only short POSTs skip this. This should knock 30 seconds
off a full run of the test suite since the 100-continue timeout will no
longer be hit.

Closes #10740
@github-actions github-actions bot added the tests label Mar 10, 2023
@bagder bagder added the HTTP label Mar 10, 2023
@dfandrich
Copy link
Contributor Author

I'm not sure what's up with tests 154 and 155. In some configurations they're failing, in others they're succeeding. The tests are both --anyauth tests with differences in the initial, auth-probing PUT. The failing tests are not logging the data in that initial request. Could it be that the server is closing the connection immediately after the headers are sent, when it recognizes that it's going to have to return 403 (and throw away the body)? Maybe there's no chance to log the body in that case.

@bagder
Copy link
Member

bagder commented Mar 10, 2023

If you get stuck on this, I can dig in a little and see if I can figure it out after the weekend.

@dfandrich
Copy link
Contributor Author

I'm able to reproduce this locally occasionally, so there's hope.

@dfandrich dfandrich force-pushed the dfandrich/shortput branch from 42edcc2 to c5de634 Compare March 10, 2023 23:15
@dfandrich
Copy link
Contributor Author

Looks like I was right. Both tests had the auth_required flag which forced the server to close the connection immediately after receipt of the request. I'm still not exactly sure why it didn't fail consistently, but (without looking at the server code) I'm guessing it had to do with timeing and how data was split in getting to the server.

@dfandrich dfandrich closed this in ee521a1 Mar 12, 2023
@dfandrich dfandrich deleted the dfandrich/shortput branch March 12, 2023 03:01
bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
This is already how curl is documented to behave in Everything curl, but
in actuality only short POSTs skip this. This should knock 30 seconds
off a full run of the test suite since the 100-continue timeout will no
longer be hit.

Closes curl#10740
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants