partially fixed #2805 (multiple http byte ranges are not yet supported)

[Fahrradkette]

Now the slice returned by Request.http_range() is also correct when there is no "start" value specified i.e. when the requester only wants the tail of the range.

Are there changes in behavior for the user?

If they rely on rng.start and rng.stop this change will break their app.

Related issue number

Partial fix for #2805, if we want to create a set or list of ranges from those exotic headers, we probably need another method name (http_multi_ranges) or something

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes
• If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • The format is <Name> <Surname>.
  • Please keep alphabetical order, the file is sorted by names.
• Add a new news fragment into the CHANGES folder
  • name it <issue_id>.<type> for example (588.bugfix)
  • if you don't have an issue_id change it to the pr id after creating the pr
  • ensure type is one of the following:
    • .feature: Signifying a new feature.
    • .bugfix: Signifying a bug fix.
    • .doc: Signifying a documentation improvement.
    • .removal: Signifying a deprecation or removal of public API.
    • .misc: A ticket has been closed, but it is not of interest to users.
  • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

[ikanobori]

What happens to end here, aren't you going to get a start of say -500 combined with an end of 500 and get to a slice(-500, 500, 1)?

Maybe end needs to be reset :)

[Fahrradkette]

This logic should make from a "-500" the slice(-500, None, 1) if I understood it correctly.

Edit: I think this PR needs more work on the receiving end of http_range, i.e. in web_fileresponse.py

I guess we're also breaking code which relies on the current slice notation

[asvetlov]

Please add a test

[codecov-io]

Codecov Report

Merging #2812 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2812      +/-   ##
==========================================
+ Coverage   97.99%   97.99%   +<.01%     
==========================================
  Files          39       39              
  Lines        7379     7380       +1     
  Branches     1296     1296              
==========================================
+ Hits         7231     7232       +1     
  Misses         47       47              
  Partials      101      101

Impacted Files	Coverage Δ
aiohttp/web_request.py	99.69% <100%> (ø)	⬆️
aiohttp/web_fileresponse.py	97.87% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c31dc9...8371204. Read the comment docs.

[Fahrradkette]

slice(None, 42, None) != slice(0, 42, 1) even though they're referencing the same data.

I'm not sure on how to test this throughly, should we rather set _payload then passing it as a request payload? If it's OK to set the payload, we could use an int-list and therefore we would have unique values in the examined range.

I also don't know which tests are required to check if it fails correctly, should I just add one which checks some chars outside ascii 48-57 to check the regex?

[asvetlov]

Everything is good, thank you.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new