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/.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 +``` 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" }, 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