Bug 212065
| Summary: | fetch -r fails on a complete file, even with -S 12345 (OS version independent) | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Matthias Andree <mandree> | ||||||
| Component: | bin | Assignee: | Dag-Erling Smørgrav <des> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Some People | CC: | cem, felixstella | ||||||
| Priority: | --- | Keywords: | patch | ||||||
| Version: | 10.3-RELEASE | Flags: | des:
mfc-stable11+
des: mfc-stable10+ |
||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
|
Description
Matthias Andree
2016-08-22 21:40:12 UTC
libfetch does appear to recognize that 416 errors might just be empty ranges:
1806 case HTTP_BAD_RANGE:
1807 /*
1808 * This can happen if we ask for 0 bytes because
1809 * we already have the whole file. Consider this
1810 * a success for now, and check sizes later.
1811 */
1812 break;
And if the HTTP server returns a Content-Range header along with the 416, it treats that as no error:
1926 /* requested range not satisfiable */
1927 if (conn->err == HTTP_BAD_RANGE) {
1928 if (url->offset == size && url->length == 0) {
1929 /* asked for 0 bytes; fake it */
1930 offset = url->offset;
1931 clength = -1;
1932 conn->err = HTTP_OK;
1933 break;
Otherwise, it treats it as an error:
1934 } else {
1935 http_seterr(conn->err);
1936 goto ouch;
1937 }
https://tools.ietf.org/html/rfc2616#section-14.16
> The Content-Range entity-header is sent with a partial entity-body to
> specify where in the full entity-body the partial body should be
> applied.
> A server sending a response with status code 416 (Requested range not
> satisfiable) SHOULD include a Content-Range field with a byte-range-
> resp-spec of "*". The instance-length specifies the current length of
> the selected resource.
The byte-range-resp-spec is the left side of the A-B/C style response of Content-Range. So httpds should respond to zero byte range requests with 416 + Content-Range: bytes */1234, if the total file size is 1234.
It appears that ec2's web server isn't following this SHOULD recommendation. That's allowed, it's not a MUST. So maybe libfetch needs to be a little more tolerant of such responses.
Created attachment 179070 [details]
Extremely naive patch to treat all 416 errors as successes
This patch probably shouldn't be applied as is. It just removes the Content-Range validation on 416 responses to range requests and treats them all as completed resumes.
I think your suggestion of following these missing "Content-Range" requests up with a HEAD request is more wise, but a less straightforward patch.
Created attachment 179140 [details]
Assume that a 416 with non-zero offset means EOF
The solution is actually quite simple.
Since 416 is an error code, any Content-Range header in the response would refer to the error message, not the requested document, so relying on the value of size when you know you got a 416 is wrong. And that scenario shouldn't happen anyway, so size should always be -1, and the if will always hit the else clause. Simply checking for url->offset > 0 on line 1928 instead of comparing it to size should fix the problem. The question is whether it introduces others; I believe the answer is no, but I'm not absolutely certain.
A commit references this bug: Author: des Date: Sun Mar 5 12:06:46 UTC 2017 New revision: 314701 URL: https://svnweb.freebsd.org/changeset/base/314701 Log: Fix partial requests (used by fetch -r) when the requested file is already complete. Since 416 is an error code, any Content-Range header in the response would refer to the error message, not the requested document, so relying on the value of size when we know we got a 416 is wrong. Instead, just verify that offset == 0 and assume that we've reached the end of the document (if offset > 0, we did not request a range, and the server is screwing with us). Note that we cannot distinguish between reaching the end and going past it, but that is a flaw in the protocol, not in the code, so we just have to assume that the caller knows what it's doing. A smart caller would request an offset slightly before what it believes is the end and compare the result to what is already in the file. PR: 212065 Reported by: mandree MFC after: 3 weeks Changes: head/lib/libfetch/http.c A commit references this bug: Author: des Date: Fri Mar 24 14:19:53 UTC 2017 New revision: 315902 URL: https://svnweb.freebsd.org/changeset/base/315902 Log: MFH (r313974,r314596): open .netrc early in case we want to drop privs MFH (r314396,r315143): fix a crash caused by an incorrect format string MFH (r314701): fix handling of 416 errors when requesting a range MFH (r315455): fix parsing of IP literals (square brackets) PR: 212065, 217723 Changes: _U stable/11/ stable/11/lib/libfetch/common.c stable/11/lib/libfetch/common.h stable/11/lib/libfetch/fetch.c stable/11/lib/libfetch/fetch.h stable/11/lib/libfetch/http.c A commit references this bug: Author: des Date: Fri Mar 24 14:26:01 UTC 2017 New revision: 315904 URL: https://svnweb.freebsd.org/changeset/base/315904 Log: MFH (r313974,r314596): open .netrc early in case we want to drop privs MFH (r314396,r315143): fix a crash caused by an incorrect format string MFH (r314701): fix handling of 416 errors when requesting a range MFH (r315455): fix parsing of IP literals (square brackets) PR: 212065, 217723 Changes: _U stable/10/ stable/10/lib/libfetch/common.c stable/10/lib/libfetch/common.h stable/10/lib/libfetch/fetch.c stable/10/lib/libfetch/fetch.h stable/10/lib/libfetch/http.c The fix that has been applied for this bug renders the -rR options meaningless in those cases where the server does not add a Content-Range header in its 416 response. In such cases, fetch re-downloads the whole file(s) which defeats the purpose of the -rR options. Please apply a more comprehensive fix for this issue. |