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

Multipart upload missing the boundary header #512

Closed
dgnthr opened this issue Nov 7, 2023 · 9 comments · Fixed by #538
Closed

Multipart upload missing the boundary header #512

dgnthr opened this issue Nov 7, 2023 · 9 comments · Fixed by #538
Labels
help wanted Extra attention is needed

Comments

@dgnthr
Copy link

dgnthr commented Nov 7, 2023

Hi,

when I generate a multipart upload I can't post it to my server due to the missing header "boundary". The header that gets generated is just 'Content-Type': 'multipart/form-data'. Using for example Insomnia to generate the same request gives something like 'Content-Type': 'multipart/form-data; boundary=---011000010111000001101001'. When using fetch directly the same behavior occurs. With fetch you have to explicitly not set the header and let the browser generate it for you. In my case this does not work:

  form.append("file", file);

  const options_ = {
    method: "PUT",
    body: form,
    headers: { "Content-Type": multipart/form-data },
  };

  return fetch("/api/v1/files/", options_)

but this does

const form = new FormData();
  form.append("file", file);

  const options_ = {
    method: "PUT",
    body: form,
  };

  return fetch("/api/v1/files/", options_)

In the second case the header is generated by the browser. Also see here or here. Some time ago this was working fine.

Thank you for your time!

@helt
Copy link

helt commented Nov 23, 2023

I was just about to open an issue about this. I can confirm this issue

@Xiphe Xiphe added the help wanted Extra attention is needed label Nov 27, 2023
@helt
Copy link

helt commented Nov 28, 2023

So, basically, we just need to remove the Content-Type header before submitting? Or would that be runnable only in the browser, but not in node?

    multipart({ body, headers, ...req }: MultipartRequestOpts) {
      if (body == null) return req;
      const data = new (defaults.formDataConstructor ||
        req.formDataConstructor ||
        FormData)();

      const append = (name: string, value: unknown) => {
        if (typeof value === "string" || value instanceof Blob) {
          data.append(name, value);
        } else {
          data.append(
            name,
            new Blob([JSON.stringify(value)], { type: "application/json" }),
          );
        }
      };

      Object.entries(body).forEach(([name, value]) => {
        if (Array.isArray(value)) {
          value.forEach((v) => append(name, v));
        } else {
          append(name, value);
        }
      });

+      // remove content-type from header, as its added when fetch serializes FormData automatically.
+     const theHeader = Object.fromEntries(Object.entries(headers||{}).filter(v => v[0] === "Content-Type"));

      return {
        ...req,
        body: data,
+       headers: theHeader,
-        headers: {
-          ...headers,
-          "Content-Type": ensureMultipartContentType(
-            String(headers?.["Content-Type"]),
-          ),        },
      };
    },

https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#multipart%2Fform-data-encoding-algorithm

@bdm-k
Copy link
Contributor

bdm-k commented Dec 10, 2023

The same issue occurs in node.

@bdm-k
Copy link
Contributor

bdm-k commented Dec 10, 2023

I removed the Content-Type header before submitting a request, but encounted an EPIPE error of the write(2). What's strange is that, according to the backend output, it seems to be working fine. Currently, I'm using open-api-mocker as the backend server. Perhaps I should consider testing with a proper server instead of the mock server provided by open-api-mocker.

Here is the output from the backend:

[INFO ] > [POST] /v2/pet/5/uploadImage
[WARN ] Missing application/json content for request body
[INFO ] < [200] {"code":1,"type":"string","message":"string"} (1 ms)

@bdm-k
Copy link
Contributor

bdm-k commented Dec 11, 2023

I used Django to confirm that removing the Content-Type header is the correct solution. I'm going to try and see if I can somehow write a test script within the scope of open-api-mocker.

@bdm-k
Copy link
Contributor

bdm-k commented Dec 12, 2023

I couldn't figure out how to test within the scope of open-api-mocker. But I'm still going to create a PR, since I believe this issue really needs a quick fix.

Xiphe added a commit that referenced this issue Dec 14, 2023
- stop setting Content-Type header for multipart requests
- support HeaderInit in request objects
- support default headers

BREAKING CHANGE:
multipart/form-data headers are not set by oazapfts anymore as these are usually automatically set by the browser/node
when this causes problems on your end you can set Content-Type=multipart/form-data manually via RequestOpts

oazapfts runtime now works with `Header` instead of plain Objects `{}`. This might affect you when you use a custom
fetch implementation and manipulate headers there

fix #512
ref #509
Xiphe added a commit that referenced this issue Dec 14, 2023
- stop setting Content-Type header for multipart requests
- support HeaderInit in request objects
- support default headers

BREAKING CHANGE:
multipart/form-data headers are not set by oazapfts anymore as these are usually automatically set by the browser/node
when this causes problems on your end you can set Content-Type=multipart/form-data manually via RequestOpts

oazapfts runtime now works with `Header` instead of plain Objects `{}`. This might affect you when you use a custom
fetch implementation and manipulate headers there

oazapfts no longer safe-guards "Content-Type" headers
in the past we discarded custom Content-Type headers that didn't start with `json` for json requests
and similarly Content-Type headers that didn't start with application/x-www-form-urlencoded for form requests
we now trust you to set the correct headers for your requests

--

fix #512
ref #509
@Xiphe
Copy link
Collaborator

Xiphe commented Dec 14, 2023

I've reworked most of our header handling and removed default content types for multipart requests.

Pre-released as oazapfts@5.0.0-headers.1 might someone be able to validate that it solves this issue?

@bdm-k
Copy link
Contributor

bdm-k commented Dec 20, 2023

I can confirm that it is working properly.

@dgnthr
Copy link
Author

dgnthr commented Jan 8, 2024

Great, the prerelease version works perfectly! Thank you!

@Xiphe Xiphe closed this as completed in #538 Jan 8, 2024
Xiphe added a commit that referenced this issue Jan 8, 2024
- stop setting Content-Type header for multipart requests
- support HeaderInit in request objects
- support default headers

BREAKING CHANGE:
multipart/form-data headers are not set by oazapfts anymore as these are usually automatically set by the browser/node
when this causes problems on your end you can set Content-Type=multipart/form-data manually via RequestOpts

oazapfts runtime now works with `Header` instead of plain Objects `{}`. This might affect you when you use a custom
fetch implementation and manipulate headers there

oazapfts no longer safe-guards "Content-Type" headers
in the past we discarded custom Content-Type headers that didn't start with `json` for json requests
and similarly Content-Type headers that didn't start with application/x-www-form-urlencoded for form requests
we now trust you to set the correct headers for your requests

--

fix #512
ref #509
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
4 participants