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