Skip to content

fix(ext/http): require genuine Response returned from Deno.serve #29106

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 26 additions & 30 deletions ext/http/00_serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,10 @@ import { InnerBody } from "ext:deno_fetch/22_body.js";
import { Event } from "ext:deno_web/02_event.js";
import {
fromInnerResponse,
InnerResponse,
newInnerResponse,
ResponsePrototype,
toInnerResponse,
} from "ext:deno_fetch/23_response.js";
import { headerListFromHeaders } from "ext:deno_fetch/20_headers.js";
import {
abortRequest,
fromInnerRequest,
Expand Down Expand Up @@ -501,15 +500,8 @@ function fastSyncResponseOrStream(
return;
}

let stream;
let body;
if (respBody.streamOrStatic) {
stream = respBody.streamOrStatic;
body = stream.body;
} else {
stream = respBody;
body = respBody;
}
const stream = respBody.streamOrStatic;
const body = stream.body;
if (body !== undefined) {
// We ensure the response has not been consumed yet in the caller of this
// function.
Expand Down Expand Up @@ -564,6 +556,7 @@ function mapToCallback(context, callback, onError) {
// 500 error.
let innerRequest;
let response;
let innerResponse: InnerResponse | undefined;
try {
innerRequest = new InnerRequest(req, context);
const request = fromInnerRequest(innerRequest, "immutable");
Expand All @@ -575,8 +568,8 @@ function mapToCallback(context, callback, onError) {

response = await callback(request, new ServeHandlerInfo(innerRequest));

// Throwing Error if the handler return value is not a Response class
if (!ObjectPrototypeIsPrototypeOf(ResponsePrototype, response)) {
innerResponse = toInnerResponse(response);
if (innerResponse === undefined) {
throw new TypeError(
"Return value from serve handler must be a response or a promise resolving to a response",
);
Expand All @@ -596,9 +589,23 @@ function mapToCallback(context, callback, onError) {
} catch (error) {
try {
response = await onError(error);
if (!ObjectPrototypeIsPrototypeOf(ResponsePrototype, response)) {

innerResponse = toInnerResponse(response);
if (innerResponse === undefined) {
throw new TypeError(
"Return value from the onError handler must be a response or a promise resolving to a response",
);
}

if (response.type === "error") {
throw new TypeError(
"Return value from onError handler must be a response or a promise resolving to a response",
"Return value from the onError handler must not be an error response (like Response.error())",
);
}

if (response.bodyUsed) {
throw new TypeError(
"The body of the Response returned from the onError handler has already been consumed",
);
}
} catch (error) {
Expand All @@ -611,14 +618,14 @@ function mapToCallback(context, callback, onError) {
error,
);
response = internalServerError();
innerResponse = toInnerResponse(response);
}
}

if (span) {
updateSpanFromResponse(span, response);
}

const inner = toInnerResponse(response);
if (innerRequest?.[_upgraded]) {
// We're done here as the connection has been upgraded during the callback and no longer requires servicing.
if (response !== UPGRADE_RESPONSE_SENTINEL) {
Expand All @@ -640,19 +647,8 @@ function mapToCallback(context, callback, onError) {
return;
}

let status;
let headers;
let body;
if (inner) {
status = inner.status;
headers = inner.headerList;
body = inner.body;
} else {
status = response.status;
headers = headerListFromHeaders(response.headers);
body = response.body;
}

const status = innerResponse.status;
const headers = innerResponse.headerList;
if (headers && headers.length > 0) {
if (headers.length == 1) {
op_http_set_response_header(req, headers[0][0], headers[0][1]);
Expand All @@ -661,7 +657,7 @@ function mapToCallback(context, callback, onError) {
}
}

fastSyncResponseOrStream(req, body, status, innerRequest);
fastSyncResponseOrStream(req, innerResponse.body, status, innerRequest);
};

if (TRACING_ENABLED) {
Expand Down
1 change: 1 addition & 0 deletions ext/node/polyfills/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import {
import { getTimerDuration } from "ext:deno_node/internal/timers.mjs";
import { serve, upgradeHttpRaw } from "ext:deno_http/00_serve.ts";
import { headersEntries } from "ext:deno_fetch/20_headers.js";
import { Response } from "ext:deno_fetch/23_response.js";
import {
builtinTracer,
ContextManager,
Expand Down
Loading