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

fix(ext/fetch) Fix request clone error in flash server #16174

Merged
merged 10 commits into from
Jan 15, 2023
39 changes: 39 additions & 0 deletions cli/tests/unit/flash_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2328,6 +2328,45 @@ Deno.test(
},
);

// https://github.com/denoland/deno/issues/15858
Deno.test(
{ permissions: { net: true } },
async function httpServerCanCloneRequest() {
const ac = new AbortController();
const listeningPromise = deferred();

const server = Deno.serve({
handler: async (req) => {
const cloned = req.clone();
assertEquals(req.headers, cloned.headers);

// both requests can read body
await req.text();
await cloned.json();

return new Response("ok");
},
signal: ac.signal,
onListen: onListen(listeningPromise),
onError: createOnErrorCb(ac),
});

try {
await listeningPromise;
const resp = await fetch("http://localhost:9000/", {
headers: { connection: "close" },
method: "POST",
body: '{"sus":true}',
});
const text = await resp.text();
assertEquals(text, "ok");
} finally {
ac.abort();
await server;
}
},
);

// Checks large streaming response
// https://github.com/denoland/deno/issues/16567
Deno.test(
Expand Down
50 changes: 44 additions & 6 deletions ext/fetch/23_request.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,11 @@
/**
* https://fetch.spec.whatwg.org/#concept-request-clone
* @param {InnerRequest} request
* @param {boolean} skipBody
* @param {boolean} flash
* @returns {InnerRequest}
*/
function cloneInnerRequest(request, skipBody = false) {
function cloneInnerRequest(request, skipBody = false, flash = false) {
const headerList = ArrayPrototypeMap(
request.headerList,
(x) => [x[0], x[1]],
Expand All @@ -168,6 +170,19 @@
body = request.body.clone();
}

if (flash) {
return {
body,
methodCb: request.methodCb,
urlCb: request.urlCb,
headerList: request.headerList,
streamRid: request.streamRid,
serverId: request.serverId,
redirectMode: "follow",
redirectCount: 0,
};
}

return {
method: request.method,
headerList,
Expand Down Expand Up @@ -487,16 +502,30 @@
}
let newReq;
if (this[_flash]) {
newReq = cloneInnerRequest(this[_flash]);
newReq = cloneInnerRequest(this[_flash], false, true);
} else {
newReq = cloneInnerRequest(this[_request]);
}
const newSignal = abortSignal.newSignal();
abortSignal.follow(newSignal, this[_signal]);

if (this[_signal]) {
abortSignal.follow(newSignal, this[_signal]);
}

if (this[_flash]) {
return fromInnerRequest(
newReq,
newSignal,
guardFromHeaders(this[_headers]),
true,
);
}

return fromInnerRequest(
newReq,
newSignal,
guardFromHeaders(this[_headers]),
false,
);
}

Expand Down Expand Up @@ -573,14 +602,22 @@

/**
* @param {InnerRequest} inner
* @param {AbortSignal} signal
* @param {"request" | "immutable" | "request-no-cors" | "response" | "none"} guard
* @param {boolean} flash
* @returns {Request}
*/
function fromInnerRequest(inner, signal, guard) {
function fromInnerRequest(inner, signal, guard, flash) {
const request = webidl.createBranded(Request);
request[_request] = inner;
if (flash) {
request[_flash] = inner;
} else {
request[_request] = inner;
}
request[_signal] = signal;
request[_getHeaders] = () => headersFromHeaderList(inner.headerList, guard);
request[_getHeaders] = flash
? () => headersFromHeaderList(inner.headerList(), guard)
: () => headersFromHeaderList(inner.headerList, guard);
return request;
}

Expand All @@ -606,6 +643,7 @@
body: body !== null ? new InnerBody(body) : null,
methodCb,
urlCb,
headerList: headersCb,
streamRid,
serverId,
redirectMode: "follow",
Expand Down
2 changes: 2 additions & 0 deletions ext/fetch/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ declare namespace globalThis {
| "request-no-cors"
| "response"
| "none",
skipBody: boolean,
flash: boolean,
): Request;
function redirectStatus(status: number): boolean;
function nullBodyStatus(status: number): boolean;
Expand Down
7 changes: 6 additions & 1 deletion ext/http/01_http.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,12 @@
false,
);
const signal = abortSignal.newSignal();
const request = fromInnerRequest(innerRequest, signal, "immutable");
const request = fromInnerRequest(
innerRequest,
signal,
"immutable",
false,
);

const respondWith = createRespondWith(
this,
Expand Down