undici icon indicating copy to clipboard operation
undici copied to clipboard

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

Open KhafraDev opened this issue 1 year ago • 1 comments

This: https://github.com/nodejs/undici/blob/af9aaea082267040aa4b3989a69abeb5e8913a27/lib/core/util.js#L461-L471

can now be replaced with: https://github.com/nodejs/undici/blob/af9aaea082267040aa4b3989a69abeb5e8913a27/lib/fetch/util.js#L1030

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.

KhafraDev avatar Dec 01 '23 21:12 KhafraDev

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

metcoder95 avatar Dec 03 '23 10:12 metcoder95

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

rossilor95 avatar Feb 19 '24 21:02 rossilor95

thats why i renamed parseRangeHeader to parseContentRangeHeader in #2779

Uzlopak avatar Feb 19 '24 22:02 Uzlopak

Cool! So, can this be closed?

rossilor95 avatar Feb 19 '24 22:02 rossilor95

yeah, naming confused me. They serve different purposes

KhafraDev avatar Feb 19 '24 22:02 KhafraDev