From 9763c0d1f8283a82b0cfdab727fa3dbe96e12f18 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Fri, 21 Aug 2026 17:48:16 -0700 Subject: [PATCH 1/3] format files outside the configured prettier globs The format scripts only cover src/**/*.ts and examples/**/*.{ts,json,jsonc}, so markdown, the prettier config itself, and the plain JS example servers had drifted from the repo style. Changes are cosmetic: trailing newlines, blank lines around markdown lists, quote style, and stray trailing whitespace. The two example server.js files were verified to parse as ES modules and to be identical modulo quotes, whitespace, and semicolons. --- .github/bonk_reviewer.md | 2 + .prettierrc | 2 +- AGENTS.md | 2 +- examples/core-tests/README.md | 2 +- .../load-balancing/container_src/server.js | 17 ++++---- .../multiple-ports/container_src/server.js | 41 ++++++++----------- examples/websocket/README.md | 2 +- 7 files changed, 32 insertions(+), 36 deletions(-) diff --git a/.github/bonk_reviewer.md b/.github/bonk_reviewer.md index e7cb0e9..03b5020 100644 --- a/.github/bonk_reviewer.md +++ b/.github/bonk_reviewer.md @@ -77,6 +77,7 @@ Read `AGENTS.md` at the repo root before reviewing. Key facts that matter for re **Outbound interception priority:** The handler-resolution order (runtime `setOutboundByHost` → static `outboundByHost` → runtime `setOutboundHandler` → static `outbound` → direct internet) is documented behavior. Flag any change that alters this precedence, removes `ContainerProxy`-export requirements, or changes the static-vs-instance lookup semantics. **Public API stability:** This is a published npm package. Anything reachable from `src/index.ts` is part of the public surface. Flag: + - Breaking signature changes to `Container`, `ContainerProxy`, `getRandom`, `getContainer`, `switchPort`, `loadBalance`, `outboundParams` - Renamed or removed lifecycle hooks, instance properties (`defaultPort`, `requiredPorts`, `sleepAfter`, `envVars`, `entrypoint`, `enableInternet`, `pingEndpoint`) - New required parameters added to existing public methods @@ -88,6 +89,7 @@ Read `AGENTS.md` at the repo root before reviewing. Key facts that matter for re **Tests:** Unit tests live in `src/tests/` (mocked container ctx). Integration tests live in `examples/*/test/` and spawn `wrangler dev` + Docker. Per `AGENTS.md`, new functionality should prefer unit tests when the behavior can be exercised via `src/tests/fixtures.ts`; only reach for an integration test when the unit fixtures cannot cover it. Flag new tests that take the integration path unnecessarily. **TypeScript discipline:** This is a TS library. Flag: + - New `any` types in public signatures - Loosened generics on public methods - Missing `await` on promise-returning calls inside `Container` methods (the DO runtime will silently lose work) diff --git a/.prettierrc b/.prettierrc index 4fc6e0d..bfc93db 100644 --- a/.prettierrc +++ b/.prettierrc @@ -7,4 +7,4 @@ "trailingComma": "es5", "bracketSpacing": true, "arrowParens": "avoid" -} \ No newline at end of file +} diff --git a/AGENTS.md b/AGENTS.md index 2c99494..2c837c2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,6 +14,7 @@ This is the `@cloudflare/containers` npm package — a TypeScript library that w - `getRandom`, `loadBalance` (deprecated), `getContainer`, `switchPort` — utility functions ## Releasing + Add a new file to the `.changeset` directory representing a new version when wrapping up changes so they get released. Do NOT change CHANGELOG.md, that is codegenerated on a release MR that is automated. @@ -175,4 +176,3 @@ This repo uses [changesets](https://github.com/changesets/changesets). When maki ```bash pnpm changeset ``` - diff --git a/examples/core-tests/README.md b/examples/core-tests/README.md index bf1d168..2ca809d 100644 --- a/examples/core-tests/README.md +++ b/examples/core-tests/README.md @@ -22,4 +22,4 @@ pnpm test - `/start` - Start container without waiting for ports - `/startAndWaitForPorts` - Start container and wait for ports to be ready - `/status` - Get container state -- `/stop` - Stop container \ No newline at end of file +- `/stop` - Stop container diff --git a/examples/load-balancing/container_src/server.js b/examples/load-balancing/container_src/server.js index ee18598..f842e4c 100644 --- a/examples/load-balancing/container_src/server.js +++ b/examples/load-balancing/container_src/server.js @@ -1,13 +1,13 @@ -import { createServer } from "http"; +import { createServer } from 'http'; const server = createServer(function (req, res) { if (req.url === '/error') { - res.writeHead(500, { "Content-Type": "text/plain" }); - res.end("Internal server error"); + res.writeHead(500, { 'Content-Type': 'text/plain' }); + res.end('Internal server error'); return; } - - res.writeHead(200, { "Content-Type": "text/plain" }); + + res.writeHead(200, { 'Content-Type': 'text/plain' }); res.end(`Hello from load balancing container!`); }); @@ -15,7 +15,6 @@ server.listen(8080, function () { console.log(`Load balancing server listening on port 8080`); }); -server.on("exit", () => { - console.log("Load balancing server exiting"); -}) - +server.on('exit', () => { + console.log('Load balancing server exiting'); +}); diff --git a/examples/multiple-ports/container_src/server.js b/examples/multiple-ports/container_src/server.js index 7bdb506..6c64bf9 100644 --- a/examples/multiple-ports/container_src/server.js +++ b/examples/multiple-ports/container_src/server.js @@ -1,45 +1,40 @@ -import { createServer } from "http"; -import { setTimeout } from "timers/promises"; +import { createServer } from 'http'; +import { setTimeout } from 'timers/promises'; const server = createServer(function (req, res) { if (req.url === '/error') { - res.writeHead(500, { "Content-Type": "text/plain" }); - res.end("Internal server error"); + res.writeHead(500, { 'Content-Type': 'text/plain' }); + res.end('Internal server error'); return; } - - res.writeHead(200, { "Content-Type": "text/plain" }); + + res.writeHead(200, { 'Content-Type': 'text/plain' }); res.end(`Hello from test container server one! process.env.MESSAGE: ${process.env.MESSAGE}`); }); server.listen(8080, function () { console.log(`Test server listening on port 8080`); }); -server.on("exit", () => { - console.log("Test server one exiting"); -}) - - - -await setTimeout(5000) +server.on('exit', () => { + console.log('Test server one exiting'); +}); +await setTimeout(5000); const server2 = createServer(function (req, res) { if (req.url === '/error') { - res.writeHead(500, { "Content-Type": "text/plain" }); - res.end("Internal server error"); + res.writeHead(500, { 'Content-Type': 'text/plain' }); + res.end('Internal server error'); return; } - - res.writeHead(200, { "Content-Type": "text/plain" }); + + res.writeHead(200, { 'Content-Type': 'text/plain' }); res.end(`Hello from test container server two! process.env.MESSAGE: ${process.env.MESSAGE}`); }); server2.listen(8081, function () { console.log(`Test server two listening on port 8081`); -}); - -server2.on("exit", function () { - console.log("Test server two exiting"); -}); - +}); +server2.on('exit', function () { + console.log('Test server two exiting'); +}); diff --git a/examples/websocket/README.md b/examples/websocket/README.md index 035f4a3..1f44ca9 100644 --- a/examples/websocket/README.md +++ b/examples/websocket/README.md @@ -24,4 +24,4 @@ pnpm test ```bash wscat -c "ws://localhost:8787/fetch/ws?id=test1" -``` \ No newline at end of file +``` From 2b65b50e7063b74148288639b6124eed14e5983a Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Fri, 21 Aug 2026 17:48:51 -0700 Subject: [PATCH 2/3] check formatting across the whole repo Point the format scripts at "." and let .prettierignore decide what is excluded, instead of maintaining a narrow glob list that silently let markdown, config, and plain JS drift. .prettierignore already covers node_modules, dist, the lockfile, generated worker types, .wrangler, .changeset, and CHANGELOG.md, so changesets and generated files stay unchecked. This widens the set of files CI enforces from TypeScript plus example JSON to also include markdown, yaml, and JS. The repo is clean under the new scope. --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 6fed4d7..bfcb0c1 100644 --- a/package.json +++ b/package.json @@ -12,8 +12,8 @@ "typecheck:examples": "pnpm --recursive --sequential --filter \"./examples/*\" run typecheck", "typecheck:all": "pnpm run typecheck && pnpm run typecheck:examples", "lint": "eslint src", - "format": "prettier --write \"src/**/*.ts\" \"examples/**/*.{ts,json,jsonc}\"", - "format:check": "prettier --check \"src/**/*.ts\" \"examples/**/*.{ts,json,jsonc}\"", + "format": "prettier --write .", + "format:check": "prettier --check .", "test": "pnpm test:unit && pnpm --recursive --sequential --filter \"./examples/*\" run test", "test:unit": "vitest run src/tests" }, From 617c08ff51f8bbfbcd12b0d32b69b397554abd62 Mon Sep 17 00:00:00 2001 From: Matt Johnson-Pint Date: Fri, 21 Aug 2026 17:45:10 -0700 Subject: [PATCH 3/3] anchor the https scheme downgrade in containerFetch The scheme downgrade applied before forwarding to the container used an unanchored string replace. String.prototype.replace rewrites only the first match, so when the request URL was already http the first https: in the path, query, or fragment was downgraded instead of the scheme. This corrupts parameters carrying an absolute URL, for example /callback?redirect=https://app.example.com, which arrives at the container as redirect=http://app.example.com. Percent-encoded values were unaffected. Relative paths resolve against http://container, so every relative-path call met the condition once #238 made them usable. Anchoring the match leaves the intended https-to-http downgrade intact, since tcpPort.fetch opens a raw TCP connection that does not terminate TLS. --- .../fix-container-fetch-scheme-downgrade.md | 8 +++++ src/lib/container.ts | 9 +++-- src/tests/container.test.ts | 36 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 .changeset/fix-container-fetch-scheme-downgrade.md diff --git a/.changeset/fix-container-fetch-scheme-downgrade.md b/.changeset/fix-container-fetch-scheme-downgrade.md new file mode 100644 index 0000000..55edad2 --- /dev/null +++ b/.changeset/fix-container-fetch-scheme-downgrade.md @@ -0,0 +1,8 @@ +--- +'@cloudflare/containers': patch +--- + +Fix `containerFetch` rewriting `https:` inside query strings and fragments. The scheme downgrade +applied to container requests used an unanchored string replace, so a URL that was already +`http:` had the first `https:` in its query or fragment downgraded instead of its scheme — +corrupting parameters that carry an absolute URL, such as `/callback?redirect=https://app.example.com`. diff --git a/src/lib/container.ts b/src/lib/container.ts index 67c8677..bde983c 100644 --- a/src/lib/container.ts +++ b/src/lib/container.ts @@ -1204,8 +1204,13 @@ export class Container extends DurableObject { const tcpPort = this.container.getTcpPort(port); - // Create URL for the container request - const containerUrl = request.url.replace('https:', 'http:'); + // Create URL for the container request. `tcpPort.fetch` opens a raw TCP connection to the + // container, which does not terminate TLS, so an https scheme has to be downgraded. + // The match is anchored on purpose: an unanchored string replace rewrites the first `https:` + // anywhere in the URL, which corrupts query strings and fragments that carry an absolute URL + // (for example `/callback?redirect=https://app.example.com`) whenever the scheme is already + // http. + const containerUrl = request.url.replace(/^https:/, 'http:'); this.inflightRequests++; diff --git a/src/tests/container.test.ts b/src/tests/container.test.ts index c2da5ff..08f48a7 100644 --- a/src/tests/container.test.ts +++ b/src/tests/container.test.ts @@ -360,6 +360,42 @@ describe('Container', () => { expect(tcpPort.fetch).toHaveBeenCalledWith('http://example.com/admin', expect.any(Request)); }); + test('containerFetch should preserve https: in query strings and fragments', async ({ + mockCtx, + container, + }) => { + mockCtx.container.running = true; + mockCtx.storage.get.mockResolvedValue({ status: 'healthy', lastChange: Date.now() }); + + await container.containerFetch('/callback?redirect=https://app.example.com#https://fragment'); + + const tcpPort = mockCtx.container.getTcpPort.mock.results[0].value; + expect(tcpPort.fetch).toHaveBeenCalledWith( + 'http://container/callback?redirect=https://app.example.com#https://fragment', + expect.any(Request) + ); + }); + + test('containerFetch should downgrade only the scheme of an https URL', async ({ + mockCtx, + container, + }) => { + mockCtx.container.running = true; + mockCtx.storage.get.mockResolvedValue({ status: 'healthy', lastChange: Date.now() }); + + await container.containerFetch( + 'https://example.com/callback?redirect=https://app.example.com', + { method: 'GET' }, + 3000 + ); + + const tcpPort = mockCtx.container.getTcpPort.mock.results[0].value; + expect(tcpPort.fetch).toHaveBeenCalledWith( + 'http://example.com/callback?redirect=https://app.example.com', + expect.any(Request) + ); + }); + test('containerFetch should return 429 when startup is rate limited', async ({ container }) => { const mockRequest = new Request('https://example.com/test', { method: 'GET' }); using startSpy = vi