RFC: Content-Length on Response should be taken into account?

node-fetch currently completely ignoring Content-Length header while consuming response.
Fetch specification about handling Content-Length on server response says almost nothing:
https://fetch.spec.whatwg.org/#concept-http-network-fetch (see whatwg/fetch#67)

On other hand, we have a fetch-node specific extension to limit the size of the response.

My proposal - analyze and throw early when size for response is specified and Content-Length of response is greater than it.

browser + node compatible with combination of both size and timeout:

class SizeLimit {
  contorller(ctrl, limit) {
    this.controller = ctrl
    this.limit = limit
    this.downloaded = 0
  }

  stopLargerResponse (res) {
    if (!res.ok) {
      // stops the timer
      this.controller.abort()
      return
    }

    const size = res.headers.get('content-length')
    if (+size > limit) this.controller.abort()
    if (encoding === 'identity') {
      this.uncompressedSize = +size
    }
  }

  count (size) {
    this.downloaded += size

    if (this.downloaded > this.limit) {
      this.controller.abort()
    }
  }
}

class Timeout {
  constructor (ctrl, time) {
    this.controller = ctrl
    this.timeout = setTimeout(() => ctrl.abort(), time)
    ctrl.signal.addEventListener('abort', () => clearTimeout(this.timeout))
  }
}

// ----------------------------------------

// create a shared abort controller
var ctrl = new AbortController()
var timeout = new Timeout(ctrl, 5000)
var sizeLimit = new SizeLimit(ctrl, 1024)
// vueConponent.onDestroy = ctrl.abort.bind(ctrl)

const res = await fetch(largeData, { signal: ctrl.signal })

sizeLimit.stopLargerResponse(res)

for await (let chunk of res.body) {
  sizeLimit.count(chunk.length)
  // do something with chunk
}

The timeout-signal is too simple for using it with something else

stumble upon a issue when i try to extend node-fetch's Request with my HttpFileLike that had a size property

got an error saying "can not assign a readonly size property" originated from body.js or somewhere from node-fetch.... it was unexpected and not what i wanted. Have never actual used the size limit in node-fetch myself...

@jimmywarting

@jimmywarting 👌 do you mean we should remove size parameter at all?

I would be okey by dropping it...

never actually used node-fetch size extension (don't believe many other ppl use it either). I mostly just see it as bloated code that most never use anyway
size don't work in browser, so it would be nice to have a cross platform solution anyway

but i know someone are going to nag on us for removing it if we don't come up with an alternativ solution. so I'm not saying yes or no yet.

Would like to know what others think first. just felt like i had to write something up to (possible) prevent unnecessary commit & test from ever getting in in the first place if we do decide to remove size before ppl start using the size extension more and more - it's often harder to remove/deprecate code

size was also one of those things that where added way back before AbortController existed and it was not possible to abort a request

(didn't see this issue as a bug per say just an enhancement to abort pre early - that never existed)

Also if i where to refactor my HttpFileLike then i probably would have removed the extend and just create a private field that had the Request internally.

Revert "Limit via header"

I don’t quite agree on this change. Anything that hides what is actually there in actual responses. Even if it’s invalid, too large a body, it should be able to be seen.

feat: check Content-Length header early when size limit is set

Navigation Menu

Repository navigation

RFC: `Content-Length` on Response should be taken into account? #879

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

RFC: Content-Length on Response should be taken into account? #879

Description

Activity

jimmywarting commented on Jun 12, 2020

jimmywarting commented on Jun 12, 2020

tinovyatkin commented on Jun 12, 2020

jimmywarting commented on Jun 12, 2020

jimmywarting commented on Jun 12, 2020

jimmywarting commented on Jun 12, 2020

jimmywarting commented on Jun 12, 2020

issuefiler commented on May 29, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

RFC: `Content-Length` on Response should be taken into account? #879