-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[miniflare/wrangler] fix: avoid "expected non-null body source" on 401 responses with a request body #13119
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
[miniflare/wrangler] fix: avoid "expected non-null body source" on 401 responses with a request body #13119
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| --- | ||
| "miniflare": patch | ||
| "wrangler": patch | ||
| --- | ||
|
|
||
| fix: avoid "expected non-null body source" error on 401 responses with a request body | ||
|
|
||
| `undici.fetch()` implements the Fetch spec's 401 credential-retry path, which throws `TypeError: fetch failed` with cause `"expected non-null body source"` whenever a request has a `ReadableStream` body and the server responds with HTTP 401. This affected `Miniflare#dispatchFetch()`, `unstable_startWorker().fetch()`, and the internal Cloudflare API client used by Wrangler. | ||
|
|
||
| The fix replaces `undici.fetch()` with `undici.request()` at the affected call sites. `undici.request()` uses the Dispatcher API directly and does not implement the Fetch spec credential-retry path, so the crash cannot occur. The wrapper explicitly replicates the behaviours of `undici.fetch()` that callers rely on: `Accept`/`Accept-Encoding` default headers, transparent decompression of gzip/deflate/brotli responses, redirect following (honouring the `redirect` mode), and correct handling of multiple `set-cookie` headers. | ||
|
|
||
| The workspace-level `pnpm` patch for `undici@7.24.4` that was previously used as a workaround has been removed. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import assert from "node:assert"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import http from "node:http"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Readable } from "node:stream"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { IncomingRequestCfProperties } from "@cloudflare/workers-types/experimental"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as undici from "undici"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { UndiciHeaders } from "undici/types/dispatcher"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -16,7 +17,23 @@ export async function fetch( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| init?: RequestInit | Request | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<Response> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const requestInit = init as RequestInit; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const request = new Request(input, requestInit); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If input is already a Request-like object and there's no extra init to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // merge, use it directly rather than re-wrapping. Re-wrapping can fail in | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // some environments (e.g. vitest forks with mixed ESM/CJS module graphs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // because undici's internal `webidl.is.Request` brand-check uses class | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // identity, and the test's ESM-resolved `Request` may be a different object | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // from the CJS-required `Request` inside this bundle. Duck-typing on the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // public `RequestInfo` interface avoids that brittleness. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isRequestLike = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| requestInit === undefined && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof input === "object" && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| input !== null && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof (input as Request).url === "string" && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof (input as Request).method === "string" && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof (input as Request).headers === "object"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const request = isRequestLike | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? (input as Request) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : new Request(input, requestInit); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle WebSocket upgrades | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -80,10 +97,162 @@ export async function fetch( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return responsePromise; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const response = await undici.fetch(request, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dispatcher: requestInit?.dispatcher, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Response(response.body, response); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Wrap in a try/catch and re-throw as TypeError("fetch failed") to match | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // the error shape that undici.fetch() produces for dispatcher-level errors | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // (e.g. when the request contains an `upgrade` header that undici.request() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // rejects with InvalidArgumentError). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return await fetchViaRequest(request, requestInit?.dispatcher); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (e instanceof TypeError) throw e; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new TypeError("fetch failed", { cause: e }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Perform an HTTP request using `undici.request()` instead of `undici.fetch()`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `undici.fetch()` implements the Fetch spec's 401 credential-retry path, which | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * crashes with "expected non-null body source" when the request body is a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * ReadableStream and the response status is 401. This is tracked upstream at | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * https://github.com/nodejs/undici/issues/4910. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `undici.request()` uses the Dispatcher API directly and has no such path. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * To maintain behavioural parity with `undici.fetch()` we explicitly handle: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Compression: send `Accept-Encoding: gzip, deflate` and decompress responses | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Redirects: follow/manual/error modes as per the Fetch spec | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - `set-cookie`: preserve multiple values using `Headers.append()` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async function fetchViaRequest( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request: Request, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dispatcher: undici.Dispatcher | undefined, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| redirectsRemaining = 20 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<Response> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { statusCode, headers: rawHeaders, body: rawBody } = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await undici.request(request.url, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| method: request.method, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Match undici.fetch() behaviour: add Accept-Encoding if the caller | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // has not already specified one, so that the workerd entry worker | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // applies the same content-encoding logic. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Match undici.fetch() defaults: it sends `Accept: */*` and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // `Accept-Encoding: gzip, deflate` when the caller hasn't set them. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // These are placed before the spread so the caller's own headers | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // take precedence. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| accept: "*/*", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "accept-encoding": "gzip, deflate", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...Object.fromEntries(request.headers), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // undici.request() accepts Node.js Readable streams but not web | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // ReadableStreams. Convert if necessary. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| body: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request.body !== null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? Readable.fromWeb( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request.body as import("node:stream/web").ReadableStream | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dispatcher, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Build a proper Headers object. undici.request() returns headers as a plain | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // object where `set-cookie` may be an array. Passing a plain object with an | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // array value to `new Response()` merges the values incorrectly, losing the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // individual cookie boundaries. Using `Headers.append()` preserves them. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const headers = new undici.Headers(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const [name, value] of Object.entries(rawHeaders)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (Array.isArray(value)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const v of value) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers.append(name, v); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (value !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers.set(name, value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Decompress response body to match undici.fetch() behaviour. undici.fetch() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // sends Accept-Encoding and transparently decompresses the response body while | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // leaving the Content-Encoding header in place. We do the same here so that | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // callers (e.g. index.ts dispatchFetch) can apply the same Content-Encoding | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // stripping logic without receiving a still-compressed body. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // rawBody.body is the web ReadableStream exposed by undici's BodyReadable. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const webStream = rawBody.body as ReadableStream | null | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const contentEncoding = rawHeaders["content-encoding"]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let responseBody: ReadableStream | null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (contentEncoding === "gzip" || contentEncoding === "x-gzip") && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| webStream != null | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseBody = webStream.pipeThrough(new DecompressionStream("gzip")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (contentEncoding === "deflate" && webStream != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Both zlib-wrapped deflate (the common HTTP interpretation) and raw deflate | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // are handled by DecompressionStream("deflate") in Node 18+. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseBody = webStream.pipeThrough( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| new DecompressionStream("deflate") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if (contentEncoding === "br" && webStream != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // "brotli" is not in the TypeScript CompressionFormat union yet, but | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Node 22+ supports it via DecompressionStream. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseBody = webStream.pipeThrough( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| new DecompressionStream("brotli" as any) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseBody = webStream ?? null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Handle redirect modes, matching undici.fetch() behaviour. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const redirectMode = request.redirect ?? "follow"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isRedirect = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| statusCode >= 300 && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| statusCode < 400 && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rawHeaders["location"] !== undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isRedirect) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (redirectMode === "error") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await rawBody.dump(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new TypeError("fetch failed"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (redirectMode === "manual") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Response( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseBody as AsyncIterable<Uint8Array> | null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { status: statusCode, headers } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // redirect === "follow" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (redirectsRemaining === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await rawBody.dump(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new TypeError("fetch failed"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const location = new URL( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rawHeaders["location"] as string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request.url | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ).toString(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await rawBody.dump(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // For 303 See Other: switch to GET and drop the body | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const redirectMethod = statusCode === 303 ? "GET" : request.method; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const redirectBody = statusCode === 303 ? null : request.body; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return fetchViaRequest( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| new Request(location, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| method: redirectMethod, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: request.headers, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| body: redirectBody, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| redirect: request.redirect, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } as RequestInit), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dispatcher, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| redirectsRemaining - 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+232
to
+244
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: You need to
Suggested change
Note: this suggestion drops the body on all redirects. For full spec compliance on 307/308, the body would need to be tee'd or buffered before the initial |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Responses with status 204/205 must have a null body (per spec). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Pass null explicitly to avoid "Invalid response status code" errors. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const noBodyStatus = statusCode === 204 || statusCode === 205; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // undici's BodyInit type doesn't list ReadableStream, but it does accept | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // AsyncIterable<Uint8Array> which ReadableStream satisfies at runtime. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Response( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| noBodyStatus ? null : (responseBody as AsyncIterable<Uint8Array> | null), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { status: statusCode, headers } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type DispatchFetch = ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,15 @@ | ||
| import events from "node:events"; | ||
| import { Readable } from "node:stream"; | ||
| import { getDockerPath } from "@cloudflare/workers-utils"; | ||
| import { fetch, Request } from "undici"; | ||
| import { request as undiciRequest, Response, Request } from "undici"; | ||
| import { startDev } from "../dev/start-dev"; | ||
| import { run } from "../experimental-flags"; | ||
| import { logger } from "../logger"; | ||
| import type { StartDevOptions } from "../dev"; | ||
| import type { EnablePagesAssetsServiceBindingOptions } from "../miniflare-cli/types"; | ||
| import type { CfModule, Environment, Rule } from "@cloudflare/workers-utils"; | ||
| import type { Json } from "miniflare"; | ||
| import type { RequestInfo, RequestInit, Response } from "undici"; | ||
| import type { RequestInfo, RequestInit } from "undici"; | ||
|
|
||
| export interface Unstable_DevOptions { | ||
| config?: string; // Path to .toml configuration file, relative to cwd | ||
|
|
@@ -245,9 +246,47 @@ export async function unstable_dev( | |
| devServer.unregisterHotKeys?.(); | ||
| }, | ||
| fetch: async (input?: RequestInfo, init?: RequestInit) => { | ||
| return await fetch( | ||
| ...parseRequestInput(address, port, input, init, options?.localProtocol) | ||
| const [url, forwardInit] = parseRequestInput( | ||
| address, | ||
| port, | ||
| input, | ||
| init, | ||
| options?.localProtocol | ||
| ); | ||
| // forwardInit is a Request object cast as RequestInit. Using | ||
| // undici.fetch(url, requestObject) triggers "expected non-null body | ||
| // source" on 401 responses when the body is a ReadableStream, because | ||
| // undici.fetch() implements the Fetch spec's 401 credential-retry path | ||
| // which checks request.body.source. undici.request() uses the | ||
| // Dispatcher API directly and has no such path. | ||
| // See: https://github.com/cloudflare/workers-sdk/issues/12967 | ||
| const forward = forwardInit as Request; | ||
| const { statusCode, headers: rawHeaders, body } = await undiciRequest( | ||
| url.toString(), | ||
| { | ||
| method: forward.method, | ||
| headers: Object.fromEntries(forward.headers), | ||
| body: | ||
| forward.body !== null | ||
| ? Readable.fromWeb( | ||
| forward.body as import("node:stream/web").ReadableStream | ||
| ) | ||
| : null, | ||
| } | ||
| ); | ||
| // Build a Headers object that preserves multiple set-cookie values. | ||
| const responseHeaders = new Headers(); | ||
| for (const [name, value] of Object.entries(rawHeaders)) { | ||
| if (Array.isArray(value)) { | ||
| for (const v of value) responseHeaders.append(name, v); | ||
| } else if (value !== undefined) { | ||
| responseHeaders.set(name, value); | ||
| } | ||
| } | ||
| return new Response(body.body, { | ||
| status: statusCode, | ||
|
Comment on lines
+264
to
+289
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 unstable_dev fetch no longer follows HTTP redirects after switch to undici.request() The Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| headers: responseHeaders, | ||
| }); | ||
| }, | ||
| waitUntilExit: async () => { | ||
| await events.once(devServer.devEnv, "teardown"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import assert from "node:assert"; | ||
| import { Readable } from "node:stream"; | ||
| import { | ||
| APIError, | ||
| getCloudflareApiBaseUrl, | ||
|
|
@@ -7,7 +8,14 @@ import { | |
| UserError, | ||
| } from "@cloudflare/workers-utils"; | ||
| import Cloudflare from "cloudflare"; | ||
| import { fetch, FormData, Headers, Request, Response } from "undici"; | ||
| import { | ||
| fetch, | ||
| request as undiciRequest, | ||
| FormData, | ||
| Headers, | ||
| Request, | ||
| Response, | ||
| } from "undici"; | ||
| import { version as wranglerVersion } from "../../package.json"; | ||
| import { logger } from "../logger"; | ||
| import { loginOrRefreshIfRequired, requireApiToken } from "../user"; | ||
|
|
@@ -73,7 +81,38 @@ export function createCloudflareClient(complianceConfig: ComplianceConfig) { | |
|
|
||
| await logRequest(request, init); | ||
|
|
||
| const response = await fetch(request.url, request); | ||
| // Using undici.fetch(url, requestObject) triggers "expected non-null | ||
| // body source" on 401 responses when the body is a ReadableStream, | ||
| // because undici.fetch() implements the Fetch spec's 401 | ||
| // credential-retry path which checks request.body.source. | ||
| // undici.request() uses the Dispatcher API directly and has no such | ||
| // path. See: https://github.com/cloudflare/workers-sdk/issues/12967 | ||
| const { statusCode, headers: rawHeaders, body } = await undiciRequest( | ||
| request.url, | ||
| { | ||
| method: request.method, | ||
| headers: Object.fromEntries(request.headers), | ||
| body: | ||
| request.body !== null | ||
| ? Readable.fromWeb( | ||
| request.body as import("node:stream/web").ReadableStream | ||
| ) | ||
| : null, | ||
| } | ||
| ); | ||
| // Build a Headers object that preserves multiple set-cookie values. | ||
| const responseHeaders = new Headers(); | ||
| for (const [name, value] of Object.entries(rawHeaders)) { | ||
| if (Array.isArray(value)) { | ||
| for (const v of value) responseHeaders.append(name, v); | ||
| } else if (value !== undefined) { | ||
| responseHeaders.set(name, value); | ||
| } | ||
| } | ||
| const response = new Response(body.body, { | ||
| status: statusCode, | ||
|
Comment on lines
+90
to
+115
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 createCloudflareClient no longer follows HTTP redirects after switch to undici.request() The replacement of Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| headers: responseHeaders, | ||
| }); | ||
| await logResponse(response); | ||
| return response; | ||
| }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Redirect handling for 301/302 POST does not convert method to GET and null the body
The Fetch spec (HTTP-redirect fetch, step 13) requires that when a 301 or 302 redirect is received and the request method is POST, the method must be changed to GET and the body set to null. The code at lines 233-234 only handles 303 (
statusCode === 303), leaving 301/302 POST redirects to keep the POST method and attempt to reuserequest.body. Since the body ReadableStream was already locked byReadable.fromWeb(request.body)at line 150 during the initial request, accessingrequest.bodyreturns a locked stream, andnew Request(location, { body: lockedStream })will throw a TypeError. This is a behavioral regression fromundici.fetch(), which correctly converted 301/302 POST to GET with a null body.Was this helpful? React with 👍 or 👎 to provide feedback.