Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use fetch utility to parse range header instead of custom one? #2484

Closed
KhafraDev opened this issue Dec 1, 2023 · 5 comments
Closed

use fetch utility to parse range header instead of custom one? #2484

KhafraDev opened this issue Dec 1, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@KhafraDev
Copy link
Member

This:

undici/lib/core/util.js

Lines 461 to 471 in af9aaea

function parseRangeHeader (range) {
if (range == null || range === '') return { start: 0, end: null, size: null }
const m = range ? range.match(/^bytes (\d+)-(\d+)\/(\d+)?$/) : null
return m
? {
start: parseInt(m[1]),
end: m[2] ? parseInt(m[2]) : null,
size: m[3] ? parseInt(m[3]) : null
}
: null

can now be replaced with:

function simpleRangeHeaderValue (value, allowWhitespace) {

I think sharing more code between fetch and the rest of undici would be nice, especially now that fetch is supported in every version of node that undici does.

@metcoder95
Copy link
Member

It is only used within RetryHandler so it should be safe to replace it; the only thing that seems missing is that it does not consider the total byte size of the response if provided. We can infer it as the sizeEnd and fallback to content-length if missed

@mcollina mcollina added the enhancement New feature or request label Dec 4, 2023
@rossilor95
Copy link
Contributor

Hi @KhafraDev @metcoder95, I was taking a look at this issue and I think these 2 functions are meant to parse different type of headers:

  • simpleRangeHeaderValue parses the Range request header (format bytes=<range-start>-<range-end>)
  • parseRangeHeader parses the Content-Range response header (format bytes <range-start>-<range-end>/<size>)

Is this correct? Thank you

@Uzlopak
Copy link
Contributor

Uzlopak commented Feb 19, 2024

thats why i renamed parseRangeHeader to parseContentRangeHeader in #2779

@rossilor95
Copy link
Contributor

Cool! So, can this be closed?

@KhafraDev
Copy link
Member Author

yeah, naming confused me. They serve different purposes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants