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

Return correct content-range header #876

Merged
merged 1 commit into from Dec 18, 2018
Merged

Conversation

PerilousApricot
Copy link
Contributor

According to the HTTP spec, the response from a "Range" request should have a
"content-range" header in the form of "range_begin-range_end/doc_length". But
xrootd currently returns the length of the range intead of the range of the whole
file. Pass in the length of the file and additionally change the printf string since the
file size is stored in a long long

According to the HTTP spec, the response from a "Range" request should have a
"content-range" header in the form of "range_begin-range_end/doc_length". But
xrootd currently returns the length of the range intead of the range of the whole
file. Pass in the length of the file and additionally change the printf string since the
file size is stored in a long long
@bbockelm
Copy link
Contributor

Just tested this on a local server:

$ curl -H 'Range: bytes=0-5' -v http://host.example.com:8000//tmp/xrootd_export/hello_world
* About to connect() to host.example.com port 8000 (#0)
*   Trying 1:2:3:4:5:6:7:8...
* Connected to host.example.com (1:2:3:4:5:6:7:8) port 8000 (#0)
> GET //tmp/xrootd_export/hello_world HTTP/1.1
> User-Agent: curl/7.29.0
> Host: host.example.com:8000
> Accept: */*
> Range: bytes=0-5
> 
< HTTP/1.1 206 Partial Content
< Connection: Keep-Alive
< Content-Length: 6
< Content-Range: bytes 0-5/14
< 

Looks good to me! Would be reasonable to squeeze this in for 4.9.0.

@ffurano
Copy link
Contributor

ffurano commented Dec 18, 2018

Absolutely agree, that was actually a bug

@ffurano ffurano merged commit dba612b into xrootd:master Dec 18, 2018
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.

None yet

3 participants