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

fetch fails given cloned request object with form data. #16570

Closed
pvormittag opened this issue Nov 8, 2022 · 6 comments
Closed

fetch fails given cloned request object with form data. #16570

pvormittag opened this issue Nov 8, 2022 · 6 comments
Labels
bug Something isn't working correctly ext/fetch related to the ext/fetch

Comments

@pvormittag
Copy link

pvormittag commented Nov 8, 2022

I'm running into fetch() throwing "Unreachable" from https://github.com/denoland/deno/blob/v1.26.1/ext/fetch/26_fetch.js#L169. This will happen any time fetch() is given a cloned request object that has form data. The issue does not occur when body is a string, but it may also fail with any of the other BodyInit types (i.e Blob, BufferSource, etc), which I haven't tested.

I can reproduce the issue by slightly altering the same example from #10002.

// Succeeds

await fetch(new Request('http://example.com').clone());
await fetch(new Request('http://example.com', { method: 'POST', body: JSON.stringify({foo:'bar'})}).clone());

// Throws "TypeError: Unreachable"

const formData = new FormData();
formData.append('foo', 'bar');
await fetch(new Request('http://example.com', { method: 'POST', body: formData }).clone());

Tested with:

deno 1.26.1 (release, aarch64-apple-darwin)
v8 10.7.193.3
typescript 4.8.3

deno 1.27.1 (release, aarch64-apple-darwin)
v8 10.8.168.4
typescript 4.8.3

@0f-0b
Copy link
Contributor

0f-0b commented Nov 8, 2022

This happens because InnerBody.clone would fail to serialize source when it is a Blob (#12067),

second.source = core.deserialize(core.serialize(this.source));

but this line expects source to be properly cloned.

ObjectPrototypeIsPrototypeOf(BlobPrototype, req.body.source)

@pvormittag
Copy link
Author

Thank you for the insight @0f-0b! Is there a potential work-around for this issue from user land code, or does this require a patch to Deno core? I presume the latter, but for my current case I'm able to except a work-around, so any further insight would be appreciated!

@0f-0b
Copy link
Contributor

0f-0b commented Nov 9, 2022

Sorry for the late reply. I encountered this problem when using ky which clones every request but allows to modify the request just before it is cloned. My workaround is to collect the request body into an ArrayBuffer so that InnerBody.source would be a serializable Uint8Array.

// request.ts
export async function toClonableRequest(req: Request): Promise<Request> {
  return req.body ? new Request(req, { body: await req.arrayBuffer() }) : req;
}
import { ky } from "./deps.ts";
import { toClonableRequest } from "./request.ts";

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

await ky.post("http://example.com/", {
  body: form,
  hooks: { beforeRequest: [toClonableRequest] },
});

// roughly equivalent to
const req = new Request("http://example.com/", {
  body: form,
  method: "POST",
  signal: AbortSignal.timeout(10000),
});
await fetch((await toClonableRequest(req)).clone());

To actually fix this issue in Deno, I suppose it would suffice to just remove the unnecessary serialization / deserialization, since InnerBody.source is immutable anyway.

@pvormittag
Copy link
Author

My workaround is to collect the request body into an ArrayBuffer so that InnerBody.source would be a serializable Uint8Array.

That makes sense and I was working down a similar path for a hopeful workaround, so thank you for sharing! I'm sure others will run into this as well.

@dsherret dsherret added bug Something isn't working correctly ext/fetch related to the ext/fetch labels Nov 17, 2022
@0f-0b
Copy link
Contributor

0f-0b commented Jul 31, 2024

This has been partially fixed in #20217. A redirected request still loses its body (see reproduction below), which will be fixed in #24812.

const portDeferred = Promise.withResolvers<number>();
await using _server = Deno.serve({
  hostname: "127.0.0.1",
  port: 0,
  onListen: ({ port }) => portDeferred.resolve(port),
  handler: (req) => {
    const url = new URL(req.url);
    if (url.pathname === "/") {
      return Response.redirect(new URL("/a", url), 307);
    }
    return new Response(req.body);
  },
});
const port = await portDeferred.promise;
const formData = new FormData();
formData.append("foo", "bar");
const res = await fetch(
  new Request(`http://127.0.0.1:${port}`, { method: "POST", body: formData })
    .clone(),
);
console.log("%o", await res.text()); // Prints "" instead of the form data.

@lucacasonato
Copy link
Member

This is fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly ext/fetch related to the ext/fetch
Projects
None yet
Development

No branches or pull requests

4 participants