Fix integer underflow in try_range_response for empty files#3566
Conversation
430699d to
d5b43a3
Compare
|
Just to add a note from spec:
So, with a file of 0 bytes, the first-pos of 0 is not less than the length, and thus is now rejected. |
|
From CI: |
d5b43a3 to
4d568ab
Compare
|
Fixed thanks. |
f86bcea to
24c5b86
Compare
darth-raijin
left a comment
There was a problem hiding this comment.
Small question, feel free to ignore it was meant as food for thought
| let total_size = metadata.len(); | ||
|
|
||
| if total_size == 0 { | ||
| return Ok((StatusCode::RANGE_NOT_SATISFIABLE, "Range Not Satisfiable").into_response()); |
There was a problem hiding this comment.
Question:
I noticed that the error body "Range Not Satisfiable" is hard-coded directly into the response. Would it make sense to extract this into a shared constant so tests can assert on the exact same value?
Having the literal inline makes tests fragile, since they need to duplicate the exact string. If the message ever changes, that could also unintentionally break consumers relying on the current wording or cause test drift.
In particular, there are two branches in this PR that both return StatusCode::RANGE_NOT_SATISFIABLE. It might be useful if the test also asserted on the error body, to ensure the StatusCode::RANGE_NOT_SATISFIABLE comes from the branch in file_stream.rs with the expected response body, and not the test branch.
Thanks to @mufeedvh for reporting this bug.