Don't retry requests with CURLOPT_NOBODY except for HTTP retries #1243

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@Marwes
Marwes commented Feb 3, 2017 edited

This reverts commit 31e33a9.

Using sftp to delete a file with CURLOPT_NOBODY set with a reused connection would fail as curl expected to get some data. Thus it would retry the command again which fails as the file has already been deleted.

This issue had originally been fixed in 12f5c67 which I found out about from https://curl.haxx.se/mail/lib-2006-02/0029.html. However the fix got removed in 31e33a9. A straight revert as this PR does currently is perhaps not sufficient as there should at least be a comment explaining the reason for the !data->set.opt_no_body check (should it perhaps only be for FTP to not interfere with HTTP as mentioned in 31e33a9?) so I'd be happy to amend this PR with a better fix.

@mention-bot

@Marwes, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bagder, @dfandrich and @yangtse to be potential reviewers.

@jay jay added the SCP/SFTP label Feb 3, 2017
@bagder
Member
bagder commented Feb 4, 2017

I think we probably should make that rule (no_body && !using-HTTP) so that it gets retried for the HTTP case, but not for other protocols.

@Marwes
Marwes commented Feb 4, 2017

Ok, I think I got that and the explanation correct now but feel free to change it.

Markus Don't retry requests with CURLOPT_NOBODY except for HTTP retries
Using sftp to delete a file with CURLOPT_NOBODY set with a reused connection would fail as curl expected to get some data. Thus it would retry the command again which fails as the file has already been deleted.
59ad111
@Marwes Marwes changed the title from Revert "HTTP: retry failed HEAD requests too" to Don't retry requests with CURLOPT_NOBODY except for HTTP retries Feb 6, 2017
@bagder bagder self-assigned this Feb 7, 2017
@bagder bagder added a commit that closed this pull request Feb 7, 2017
@bagder Markus Westerlind + bagder transfer: only retry nobody-requests for HTTP
Using sftp to delete a file with CURLOPT_NOBODY set with a reused
connection would fail as curl expected to get some data. Thus it would
retry the command again which fails as the file has already been
deleted.

Fixes #1243
6ffe0f5
@bagder bagder closed this in 6ffe0f5 Feb 7, 2017
@bagder
Member
bagder commented Feb 7, 2017

thanks!

@Marwes Marwes deleted the Marwes:fix_sftp_reused_connection_rm branch Feb 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment