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

Doesn't work either with latest stable and beta version of node-fetch #94

Closed
octet-stream opened this issue May 13, 2021 · 6 comments
Closed

Comments

@octet-stream
Copy link
Contributor

@jimmywarting As I mentioned here, I'm running into the problem when using this package with node-fetch. The problem remains both in the latest stable and beta. With fetch-blob@3.0.0-rc.0 and fetch-blob@2.1.1.

I was trying to test an example for my form-data-encoder where the encoder targeting Blob as you did in formdata-polyfill:

import {Readable} from "stream"

import {FormData, fileFromPath} from "formdata-node"
import {Encoder} from "form-data-encoder"

import Blob from "fetch-blob"
import fetch from "node-fetch"

async function toBlob(form) {
  const encoder = new Encoder(form)
  const chunks = []

  for await (const chunk of encoder) {
    chunks.push(chunk)
  }

  return new Blob(chunks, {type: encoder.contentType})
}

const fd = new FormData()

fd.set("name", "John Doe")
fd.set("avatar", await fileFromPath("avatar.png"))

const options = {
  method: "post",
  body: await toBlob(fd)
}

const response = await fetch("https://httpbin.org/post", options)

console.log(await response.text())

When I run this code node test.mjs this happens:

  1. With fetch-blob I get the following error: body.stream().pipe(dest);. This error appears in node-fetch/src/body.js:374:17
  2. When I run the same code, but with fetch-blob@2.1.1 it will lowercase the value of Blob#type which breaks boundary string returned by form-data-encoder.
@jimmywarting
Copy link
Contributor

jimmywarting commented May 13, 2021

Historically blob have always lowercased the type both in the constructor and in the blob.slice() method (so that is what we have also always done in fetch-blob - up until now).

(i know buffer.Blob in node v15.7 and up also lowercase the blob.type - i have already warn them about this and said it might not be such a good idea to do it and fetch-blob was thinking about removing this casting to lowercase)

Like you i also had this exact same issue 3years ago that i fixed by making the boundary only lowercased jimmywarting/FormData#17

looking at your code you also use uppercase characters
My recommendation to you is that you follow the same practise as FF and i did, by lowercasing the generated boundary so it dosen't have any effect when blob lowercase the type

- `FormDataBoundary${randomBytes(16).toString("hex")}`
+ `formdata-boundary${randomBytes(16).toString("hex")}`

If the users have a option to define there own boundary in the encoder constructor then i think that you should lowercase the boundary before it starts encoding

  constructor(form: FormDataLike, boundary: string = createBoundary()) {
    if (!isFormData(form)) {
      throw new TypeError("Expected first argument to be a FormData instance.")
    }

    if (typeof boundary !== "string") {
      throw new TypeError("Expected boundary to be a string.")
    }

-    this.boundary = boundary
+    this.boundary = boundary.toLowerCase()

cuz they might use buffer.Blob or some other that also lowercase the type

I reported this lowercasing issue at whatwg/html#6251 in late 2020 and they seems to agree that lowercasing the blob's type isn't that good as you loose the parameters "real" values in the blob.type officially (i think) they haven't changed the spec around that part yet. But what i think they are planing on doing is to make the hole type lowercased appart from the values... Meaning mimetype & keys are lowercase but the value isn't. (mime/type; key=VALUE)

I stripped this lowercase casting out of fetch-blob@3.0.0-rc.0 beta so it dose less harm and loosen it up a bit so it would be weird if you had this issue with the beta


could you try if this works code works with fetch-blob@3.0.0-rc.0 ?

import {FormData} from 'formdata-node'
import {Encoder} from 'form-data-encoder'
import Blob from "fetch-blob"

const encoder = new Encoder(new FormData())
const type = encoder.contentType
console.assert(new Blob([], { type }).type === type, "type stays the same")
console.assert(new Blob([], { type: 'foo/bar; key=VALUE' }).type === 'foo/bar; key=VALUE', "type stays the same")

@octet-stream
Copy link
Contributor Author

form-data-encoder allows you to set your own boundary string as the second parameter in constructor. Tried this with nanoid, in that case the request is being send, but result remains the same. After your comment I remembered that their default alphabet has both capitalized and lowercased symbols. So, the problem with fetch-blob@2 can definitely be solved the way you described, thanks 👍
Still, with fetch-blob doesn't seem to work with async iterables. But as far as I can tell, you're already working on it. That said, my problem is solved. Thank you again!

@octet-stream
Copy link
Contributor Author

If the users have a option to define there own boundary in the encoder constructor then i think that you should lowercase the boundary before it starts encoding

Maybe I will do it to fix possible issues, or maybe I'll just recommend to do so if anybody runs into the same problem with this example.

@octet-stream
Copy link
Contributor Author

I stripped this lowercase casting out of fetch-blob@3.0.0-rc.0 beta so it dose less harm and loosen it up a bit so it would be weird if you had this issue with the beta

Tested this with v3 – the type property stays the same as the original Encoder#contentType, so no such issue with v3

@jimmywarting
Copy link
Contributor

Still, with fetch-blob doesn't seem to work with async iterables. But as far as I can tell, you're already working on it. That said, my problem is solved. Thank you again!

blob#stream is iterable. Like i mention earlier, you may have a problem with checking if it's a buffer instead of uint8array

@octet-stream
Copy link
Contributor Author

octet-stream commented May 14, 2021

No, it's just me misspelled fetch-blob with node-fetch. I meant that node-fetch does not support Symbol.asyncIterator as request body yet. And as you can see in the top of this thread, node-fetch is trying to call .pipe() method on body.stream() which does not exists in async iterator in the first place.

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

2 participants