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

FormData.hasKnownLength always returns true #399

Open
cchudant opened this issue Sep 9, 2018 · 8 comments
Open

FormData.hasKnownLength always returns true #399

cchudant opened this issue Sep 9, 2018 · 8 comments

Comments

@cchudant
Copy link

cchudant commented Sep 9, 2018

FormData.hasKnownLength() returns true whether my form data has a known length or not (ie. inserting a stream)
This break node-fetch, because they use the known length to send Content-Length header.

@alexindigo
Copy link
Member

Do you mind to elaborate on how it break node-fetch?
And when there is no known content-length, I'm not sure how it could set Content-Length header.

@cchudant
Copy link
Author

cchudant commented Sep 12, 2018

Try this:

const fetch = require('node-fetch')
const FormData = require('form-data')
const { Readable } = require('stream')

let i = 0
const stream = new Readable({
  read(size) {
    i++
    this.push(i <= 16 ? Buffer.alloc(size) : null) // Buffer full of 0's
  }
})

const apiUrl = 'https://bayfiles.com/api/upload'

const form = new FormData()
form.on('data', buf => console.log(buf.length, buf))

// form.hasKnownLength = () => false
console.log(`Form has known length: ${form.hasKnownLength()}`)

form.append('file', stream, 'myFile')

fetch(apiUrl, {
  method: 'POST',
  body: form
})
  .then(res => res.text())
  .then(json => {
    console.log(json)
  })

FormData says the stream has a known length even though it hasn't.
The upload doesn't work because a wrong Content-Length header is sent.

Then try uncommenting form.hasKnownLength = () => false; the Content-Length header is not sent and the upload works.

@cchudant
Copy link
Author

cchudant commented Oct 1, 2018

any update on that?

@yordis
Copy link

yordis commented Oct 17, 2018

I believe that form-data package shouldn't expose that function since it doesn't exist on FormData spec.

@cchudant
Copy link
Author

This function is needed for node-fetch to work.
node-fetch need to know the content length in order to send the Content-Length header when the length is available.

@yordis
Copy link

yordis commented Oct 24, 2018

@SkyBeastMC I don't know the details to be honest, but even when that is the case, it is leaking abstractions.

It is fine if node-fetch ask for all the entries and figure out Content-Length based on the form values or something like that.

But if you have to expose a function for that,

  1. Leak abstraction
  2. It is not following the spec

So probably things need to be clean up.

Just a feedback.

@dalcde
Copy link

dalcde commented Aug 1, 2020

I believe the offending line is https://github.com/form-data/form-data/blob/master/lib/form_data.js#L104 . When value is a body returned from node-fetch, the condition evaluates to true, because

body.path == undefined
value.readable == true
value.hasOwnProperty('httpVersion') == false

The body Readable Stream is then considered to have known length. So the form.hasKnownLength would incorrectly return true, and the "known" length is 0. This means node-fetch gets an incorrect Content-Length, and webservers get confused.

@nytamin
Copy link

nytamin commented Jan 27, 2021

I spent a fair bit of time troubleshooting a similar issue, the PR #382 solved the issue for me.

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

No branches or pull requests

5 participants