updated logic for making a range request with no end value

[danielRicaud]

No description provided.

[elizabethhealy]

you might want to add an extra else for when byteEnd is undefined and throw an error there, or log the error?

[dmihalcik-virtru]

Required fix: Update line 103 to end with - and remove the - from line 108. Also, 0 is always an invalid byteEnd for the current implementation, the guard on 104 should be !byteEnd || byteEnd < 0.

Actually reading the range header docs was informative. Notably, it does not implement the full slice API, so we'd need to update this to first run a request to see how big the file is, allowing us to implement the negative end output. For now we can throw an Unsupported error as above and when/if we start seeing that we can go back and add it.

[danielRicaud]

Not sure we should automatically add - to line 103 since negative range's shouldn't append the -. I've changed 107 to not allow byteEnd to be a zero value.

[danielRicaud]

Also in the case of byteEnd being undefined that's ok in the case of negative range headers like -1000

[dmihalcik-virtru]

So if byteEnd exists, it must:

• not be negative
• byteStart can't be negative either, otherwise we need to use the suffix format (last N bytes, -N)
• must be strictly greater than byteStart

So it should be like:

  if (byteEnd) {
    if (byteStart < 0 || byteEnd <= byteStart) {
      throw Error(`invalid HTTP range [${byteStart}-${byteEnd}]`);
    }
    rangeHeader += `-${(byteEnd || 0) - 1}`;
  }

[dmihalcik-virtru]

details: https://httpwg.org/specs/rfc9110.html#rfc.section.14.1.1

[dmihalcik-virtru]

Suggestion: Pull this out as an exported function, just so you can add some unit tests

[danielRicaud]

@dmihalcik-virtru implemented the changes. I'll make a quick Jira ticket to pull this function out and add unit tests just because it needs to be released as soon as possible.

[danielRicaud]

I removed this logic back to the original implementation because throwing an error that early was causing a unit test to fail that's already making sure the Range is satisfiable. On line 92 if an Unsatisfiable range is encountered with an HTTP response code of 416 the test catches the issue correctly

[dmihalcik-virtru]

Required fix: Update line 103 to end with - and remove the - from line 108. Also, 0 is always an invalid byteEnd for the current implementation, the guard on 104 should be !byteEnd || byteEnd < 0.

Actually reading the range header docs was informative. Notably, it does not implement the full slice API, so we'd need to update this to first run a request to see how big the file is, allowing us to implement the negative end output. For now we can throw an Unsupported error as above and when/if we start seeing that we can go back and add it.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication