reiern70 opened a new pull request, #1695:
URL: https://github.com/apache/wicket/pull/1695

   Closes #1676
   
   `AbstractResource` understood only the `<first>-<last>` form of a `Range` 
header, so the other byte range forms of RFC 9110 were answered wrongly.
   
   ### Before and after
   
   curl against Jetty, on the 77966 bytes of `wicket-ajax-jquery.js`:
   
   | `Range` | master | this PR | RFC 9110 |
   |---|---|---|---|
   | `bytes=-5` | 206, `bytes 0-5/77966`, the first 6 bytes | 206, `bytes 
77961-77965/77966`, the last 5 bytes | suffix range, §14.1.2 |
   | `bytes=-999999` | 206, `bytes 0-999999/77966` | 206, `bytes 0-77965/77966` 
| §14.1.2 |
   | `bytes=0-99999999` | 206, `bytes 0-99999999/77966` | 206, `bytes 
0-77965/77966` | last-pos below the length, §14.4 |
   | `bytes=20-10` | 206, `bytes 20-10/77966`, byte 20 to the end | 200, the 
whole content | invalid, §14.1.1, may be ignored, §14.2 |
   | `bytes=77966-`, `bytes=999999-` | 206, `bytes 77966-77965/77966`, empty 
body | 416, `bytes */77966`, empty body | unsatisfiable, §15.5.17 |
   | `bytes=-0` | 206, `bytes 0-0/77966`, 1 byte | 416, `bytes */77966`, empty 
body | selects no bytes |
   | `bytes=-` | 206, the whole content | 200, the whole content | names 
neither a start nor a suffix length |
   
   A satisfiable `<first>-<last>` range such as `bytes=0-9` is answered as 
before.
   
   ### What it does now
   
   - The header is parsed before the content length is known, so a suffix range 
is kept in `CONTENT_RANGE_STARTBYTE` as a negative start byte. It is resolved 
against the length both where the headers are written 
(`setResponseContentRangeHeaderFields()`) and where the body is written 
(`PartWriterCallback`).
   - An invalid range is ignored and the whole content is served with 200. Its 
meta data is cleared, so it cannot inherit a range parsed earlier in the same 
request cycle.
   - An unsatisfiable range is answered with 416 and `Content-Range: bytes 
*/<length>`. `PartWriterCallback` writes nothing for it, even when the stream 
skips fewer bytes than asked, and still closes the stream it was given.
   - A range on empty content is ignored, since no `Content-Range` can describe 
a part of it.
   
   ### API changes and migration
   
   No signature changes, but the values behind the public meta data keys do:
   
   - `CONTENT_RANGE_STARTBYTE` is negative for a suffix range, where it was 0, 
and past the end of the content for `bytes=-0`.
   - Neither key is set for an invalid range, including `bytes=-`.
   
   An application that passes the values on to `PartWriterCallback`, as 
`PackageResource` and `FileSystemResource` do, needs no change. Draft wording 
for the migration guide:
   
   > `AbstractResource.CONTENT_RANGE_STARTBYTE` is negative for a suffix range 
such as `bytes=-500`, the last 500 bytes of the content, and neither 
`CONTENT_RANGE_STARTBYTE` nor `CONTENT_RANGE_ENDBYTE` is set for an invalid 
range such as `bytes=20-10`. Code that hands the values to `PartWriterCallback` 
keeps working. Code that skips bytes or writes a `Content-Range` from them 
itself has to resolve a negative start against the content length first: 
`Math.max(0, contentLength + startbyte)`.
   
   ### Not in this PR
   
   - Headers that cannot be parsed at all (`bytes=5`, `bytes=abc`) are #1672. 
Both PRs change `setRequestRangeMetaData()`, so whichever is merged second 
needs a rebase.
   - Of several ranges only the first is served (WICKET-5995), unchanged.
   - When a resource does not know its content length, the range is still 
applied to the body of a 200 that carries no `Content-Range`. That predates 
this change.
   
   ### Tests
   
   - `PackageResourceReferenceTest#testContentRangeResponse` checks status, 
`Content-Range` and body for each form above, and `testContentRange` gets the 
new suffix and out-of-range bodies. The new `PartWriterCallbackTest` checks 
that an unsatisfiable range closes the stream and writes nothing when the 
stream skips short. All four tests fail on master.
   - The table above comes from curl against Jetty, once with master's classes 
and once with this branch.
   - `mvn clean verify -Pjs-test` is green.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to