On Fri, 31 Oct 2025 11:23:49 GMT, Daniel Fuchs <[email protected]> wrote:
>> Hello @dfuch, >> >> Thank you for the review. I have completed all the requested changes: >> formatting fixes, additional unit tests, and updates to the JavaDoc of >> `SimpleFileServer`. >> >> Regarding the CSR, I am not listed in the Census and therefore do not have a >> JBS/OpenJDK account to proceed with it. >> Would it be possible for you to either issue an account for me or handle the >> CSR on your side? >> If either option works, I can provide the necessary information, including >> the full text for the CSR ticket. >> >> Thank you very much for your understanding. > >> Regarding the CSR, I am not listed in the Census and therefore do not have a >> JBS/OpenJDK account to proceed with it. > > No problem - one of us will create the CSR, when we have setlled on the > documentation and implementation changes and the PR is approved or close to > be approved. Thanks @dfuch, @fandreuz, @Michael-Mc-Mahon and @efge for the detailed reviews. Since the discussion has become quite long, here’s a summary of the feedback and how it was addressed. --- #### @dfuch > ETag is an entirely separate feature from Range and should be in its own > separate follow-up Jira ticket and PR. -> Removed ETag-related code and tests in a separate commit. > Support for Range header will need to be documented in > `SimpleFileServer.java`. -> Updated the documentation to mention HTTP Range request support. > I would expect a validation of the ranges list before we serve them. -> Implemented range list normalization: overlapping or unsorted ranges are merged and sorted before returning, following [RFC 9110 §15.3.7.2](https://www.rfc-editor.org/rfc/rfc9110.html#section-15.3.7.2-3). --- #### @Michael-Mc-Mahon > We probably should send the `Accept-Ranges` header in response to a HEAD > request also. -> Confirmed that `Accept-Ranges` is already included for HEAD requests. > Do we need to support `If-Range` with this PR? … It seems like it belongs > more with a general conditional-requests feature. -> Removed the initial `If-Range` + ETag handling; this will be covered by a future follow-up. > Is this not an error situation, if we reach EOF and have only partially > returned the requested range? -> Updated the implementation to throw an `EOFException` and close the connection when EOF is reached before completing the requested range. --- #### General > General comments about formatting and tests -> Fixed formatting, added more tests, and improved range header parsing and validation (ensuring ranges are sorted and merged if overlapping). --- ### Still open - CSR creation/approval (maintainer side) - Clarify whether `If-Range` will remain partial (Last-Modified based) or gain full ETag support later - Final review of the range-merging logic and corresponding tests ------------- PR Comment: https://git.openjdk.org/jdk/pull/28021#issuecomment-3496435676
