From 27ef44a01fa84f363b56b0e762da3db05ed8fa50 Mon Sep 17 00:00:00 2001 From: George Moon Date: Sat, 31 Oct 2026 19:18:23 -0400 Subject: [PATCH 1/4] feat: public key-addressed web seam fetchWebKey(key, request) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promotes the internal key-addressed path to the public web API. The caller names the S3 key; the Request contributes only method (GET/HEAD) and conditional/Range headers — the request URL plays no role in key selection. fetchWeb() now derives the key from the URL and delegates to fetchWebKey(), so both share one HTTP <-> S3 translation path. Lets consumers with a non-identity URL->key mapping (a prefix, a /files/* mount) avoid round-tripping the key through URL encode/decode — which otherwise corrupts keys containing %, +, spaces, or other reserved characters unless the consumer's encoder and s3proxy's decoder agree exactly. The key is used verbatim. No behavior change to fetchWeb (same key derivation, same Response). Adds a fetchWebKey test suite incl. the reserved-character case that fetchWeb can't carry via the URL, plus README narrative + API-reference docs. Closes #162. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 34 +++++++++++++++++++ src/index.ts | 39 +++++++++++++++++++--- test/fetch-web.test.ts | 70 +++++++++++++++++++++++++++++++++++++++- test/helpers/aws-mock.ts | 10 ++++++ 4 files changed, 148 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index a53ea10..3f6e84d 100644 --- a/README.md +++ b/README.md @@ -339,6 +339,31 @@ const { stream, status, headers } = await proxy.fetch({ return new Response(Readable.toWeb(stream) as ReadableStream, { status, headers }); ``` +#### Custom URL → key mapping (`fetchWebKey`) + +When the URL path is not the S3 key — a bucket mounted under `/files/*`, or +keys living under a prefix — use **`proxy.fetchWebKey(key, request)`**. The +caller names the key; the `Request` still supplies the method and +conditional/`Range` headers, and you get the same `Response` with the same HTTP +translation as `fetchWeb`: + +```typescript +// Serve s3://my-bucket/assets/ under the /files/* route. +app.on(['GET', 'HEAD'], '/files/*', (c) => { + const key = `assets/${c.req.path.slice('/files/'.length)}`; + return proxy.fetchWebKey(key, c.req.raw); +}); +``` + +This avoids round-tripping the key through URL encode/decode — the subtle bug +where a key containing `%`, `+`, spaces, or other reserved characters serves the +wrong object (or 404s) unless your encoder and s3proxy's decoder agree exactly. +The key is used verbatim. + +> If you don't need an HTTP `Response` at all (backend processing, ETL, an API +> shaping its own headers), use the AWS SDK directly — s3proxy adds nothing +> there. Its job is serving S3 objects over HTTP, correctly. + > **Performance note.** On Node, Hono goes through `@hono/node-server` > which converts `IncomingMessage` and `Web Request/Response` on every > request, two extra layers compared with Express's direct @@ -507,6 +532,15 @@ so your framework's error handler owns the error-body format. HEAD responses have a `null` body. Uses the runtime's global `Request` / `Response`; no extra dependency. +##### `await proxy.fetchWebKey(key: string, request: Request): Promise` +Key-addressed sibling of `fetchWeb`: the caller supplies the S3 `key` and the +`Request` contributes only the method and conditional/`Range` headers — the URL +is not consulted for key selection. Same HTTP translation and typed-error +behavior as `fetchWeb`. Use it when your URL → key mapping is not identity (a +prefix, a mount point); it avoids the URL encode/decode round-trip that corrupts +keys containing `%`, `+`, spaces, or other reserved characters. The `key` is +used verbatim. + ##### `await proxy.healthCheck(): Promise` Verify bucket connectivity. Resolves on success, throws a typed `S3ProxyError` on failure. Use it from a `/health` or `/ready` handler diff --git a/src/index.ts b/src/index.ts index ff8f805..6c2e5a7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -102,14 +102,45 @@ export class S3Proxy extends EventEmitter { * `Response` yourself. */ public async fetchWeb(request: Request): Promise { - const { stream, status, headers } = await this.fetch({ + // URL picks the key: derive it from the request path (percent-decoded by + // parseRequest), then hand off to the key-addressed seam. The query string + // plays no role in key selection. + const key = parseRequest({ url: request.url, - // Pass the parsed pathname as `path` so parseRequest uses it directly - // rather than re-parsing the URL. The S3 key derives from the path - // alone, so the query string is irrelevant here. path: new URL(request.url).pathname, method: request.method, headers: Object.fromEntries(request.headers), + }).key; + return this.fetchWebKey(key, request); + } + + /** + * Key-addressed sibling of `fetchWeb`: the *caller* names the S3 key and the + * `Request` contributes only the method (GET/HEAD) and conditional/`Range` + * headers — the request URL plays no role in key selection. Returns the same + * Web `Response` with the same HTTP <-> S3 translation as `fetchWeb` + * (`Range` -> 206, missing/denied -> typed 404/403, header mapping, HEAD + * semantics, Node -> Web stream), and likewise throws the typed + * `S3ProxyError` on classified failures rather than rendering them. + * + * Use this when your URL -> key mapping is not identity (e.g. a bucket + * mounted under `/files/*`). It avoids round-tripping the key through URL + * encode/decode, which otherwise forces your encoder and s3proxy's decoder to + * agree exactly for keys containing `%`, `+`, spaces, or other reserved + * characters — or the wrong object is served / a valid key 404s. The key is + * used verbatim as the S3 object key. + * + * If you do not need an HTTP `Response` at all (backend processing, ETL, an + * API shaping its own headers), use the AWS SDK directly — s3proxy adds + * nothing there. s3proxy's job is serving S3 objects over HTTP, correctly. + */ + public async fetchWebKey(key: string, request: Request): Promise { + // The URL is unused for key selection here; fetchKey reads only the method + // and Range header. Passing request.url keeps the HttpRequest well-formed. + const { stream, status, headers } = await this.fetchKey(key, { + url: request.url, + method: request.method, + headers: Object.fromEntries(request.headers), }); // No body for HEAD; otherwise adapt the Node stream to a Web stream. // Backpressure propagates through the wrapper, so nothing buffers. diff --git a/test/fetch-web.test.ts b/test/fetch-web.test.ts index e99232a..68a0d85 100644 --- a/test/fetch-web.test.ts +++ b/test/fetch-web.test.ts @@ -1,6 +1,6 @@ import { GetObjectCommand, S3ServiceException } from '@aws-sdk/client-s3'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { S3Forbidden, S3NotFound, S3Proxy } from '../src/index.js'; +import { InvalidRequest, S3Forbidden, S3NotFound, S3Proxy } from '../src/index.js'; import { s3Mock, setupS3Mocks, teardownS3Mocks } from './helpers/aws-mock.js'; import { catchError } from './helpers/http-mocks.js'; @@ -69,3 +69,71 @@ describe('proxy.fetchWeb', () => { expect(err).toBeInstanceOf(S3Forbidden); }); }); + +describe('proxy.fetchWebKey', () => { + let proxy: S3Proxy; + + beforeEach(async () => { + setupS3Mocks(); + proxy = new S3Proxy({ bucket: '.test-bucket' }); + await proxy.init(); + }); + + afterEach(() => { + teardownS3Mocks(); + }); + + it('serves the caller-supplied key, ignoring the request URL', async () => { + // URL path is "/" but the key argument selects index.html. + const res = await proxy.fetchWebKey('index.html', new Request(url('/'))); + expect(res).toBeInstanceOf(Response); + expect(res.status).toBe(200); + expect(res.headers.get('content-type')).toBe('text/html'); + expect((await res.text()).length).toBe(338); + }); + + it('is behaviorally identical to fetchWeb when the key matches the path', async () => { + const viaKey = await proxy.fetchWebKey('index.html', new Request(url('/ignored'))); + const viaUrl = await proxy.fetchWeb(new Request(url('/index.html'))); + expect(viaKey.status).toBe(viaUrl.status); + expect(viaKey.headers.get('content-type')).toBe(viaUrl.headers.get('content-type')); + expect(viaKey.headers.get('content-length')).toBe(viaUrl.headers.get('content-length')); + }); + + it('honors the Range header from the request → 206 + content-range', async () => { + const res = await proxy.fetchWebKey( + 'index.html', + new Request(url('/'), { headers: { range: 'bytes=0-100' } }) + ); + expect(res.status).toBe(206); + expect(res.headers.get('content-range')).toBe('bytes 0-100/338'); + expect((await res.text()).length).toBe(101); + }); + + it('returns a null body for HEAD but keeps status and headers', async () => { + const res = await proxy.fetchWebKey('index.html', new Request(url('/'), { method: 'HEAD' })); + expect(res.status).toBe(200); + expect(res.body).toBeNull(); + expect(res.headers.get('content-length')).toBe('338'); + }); + + it('throws the typed S3NotFound for a missing key', async () => { + await expect( + proxy.fetchWebKey('nonexistent-file.txt', new Request(url('/'))) + ).rejects.toBeInstanceOf(S3NotFound); + }); + + // The point of the seam: a key with a literal '%' is served verbatim, where + // routing it through a URL (fetchWeb) fails because the path can't be decoded. + it('serves a key with reserved characters verbatim', async () => { + const res = await proxy.fetchWebKey('reports/q1 50%.txt', new Request(url('/'))); + expect(res.status).toBe(200); + expect(await res.text()).toBe('fifty percent'); + }); + + it('contrast: the same key via fetchWeb (URL-addressed) fails to decode', async () => { + await expect(proxy.fetchWeb(new Request(url('/reports/q1 50%.txt')))).rejects.toBeInstanceOf( + InvalidRequest + ); + }); +}); diff --git a/test/helpers/aws-mock.ts b/test/helpers/aws-mock.ts index 80ff1e0..512aa1e 100644 --- a/test/helpers/aws-mock.ts +++ b/test/helpers/aws-mock.ts @@ -217,6 +217,16 @@ The public repo is here. }, }); + // Mock an object whose key contains a literal '%' — a key the URL-addressed + // path can't carry safely (decodeURIComponent on the path throws), but the + // key-addressed seam (fetchWebKey) serves verbatim. + s3Mock.on(GetObjectCommand, { Bucket: '.test-bucket', Key: 'reports/q1 50%.txt' }).resolves({ + Body: mockBody('fifty percent'), + ContentLength: 13, + ContentType: 'text/plain', + $metadata: { httpStatusCode: 200, requestId: 'mock-request-id' }, + }); + // Mock range requests for large.bin (for MockExpress test) s3Mock .on(GetObjectCommand, { From c676f6df1ed63725c0d2291a642a70100efc67d5 Mon Sep 17 00:00:00 2001 From: George Moon Date: Sat, 31 Oct 2026 20:08:36 -0400 Subject: [PATCH 2/4] refactor: dedupe web->HttpRequest construction in fetchWeb/fetchWebKey Factor the shared { url, method, headers } lift into a private webToHttpRequest() helper so the method/header conversion lives in one place (clears Codacy's duplicate-block findings introduced by fetchWebKey). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/index.ts | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/index.ts b/src/index.ts index 6c2e5a7..3093db0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -106,14 +106,24 @@ export class S3Proxy extends EventEmitter { // parseRequest), then hand off to the key-addressed seam. The query string // plays no role in key selection. const key = parseRequest({ - url: request.url, + ...this.webToHttpRequest(request), path: new URL(request.url).pathname, - method: request.method, - headers: Object.fromEntries(request.headers), }).key; return this.fetchWebKey(key, request); } + /** + * Adapt a WHATWG `Request` to the internal `HttpRequest` shape. Shared by the + * two web entry points so the method/header lift lives in one place. + */ + private webToHttpRequest(request: Request): HttpRequest { + return { + url: request.url, + method: request.method, + headers: Object.fromEntries(request.headers), + }; + } + /** * Key-addressed sibling of `fetchWeb`: the *caller* names the S3 key and the * `Request` contributes only the method (GET/HEAD) and conditional/`Range` @@ -135,13 +145,9 @@ export class S3Proxy extends EventEmitter { * nothing there. s3proxy's job is serving S3 objects over HTTP, correctly. */ public async fetchWebKey(key: string, request: Request): Promise { - // The URL is unused for key selection here; fetchKey reads only the method - // and Range header. Passing request.url keeps the HttpRequest well-formed. - const { stream, status, headers } = await this.fetchKey(key, { - url: request.url, - method: request.method, - headers: Object.fromEntries(request.headers), - }); + // fetchKey reads only the method and Range header; the URL carried by + // webToHttpRequest is unused for key selection but keeps the shape valid. + const { stream, status, headers } = await this.fetchKey(key, this.webToHttpRequest(request)); // No body for HEAD; otherwise adapt the Node stream to a Web stream. // Backpressure propagates through the wrapper, so nothing buffers. const body = request.method === 'HEAD' ? null : (Readable.toWeb(stream) as ReadableStream); From 23ee2344ec5418ffe637db34171ed0c25f6061af Mon Sep 17 00:00:00 2001 From: George Moon Date: Sun, 1 Nov 2026 10:03:14 -0500 Subject: [PATCH 3/4] refactor: address /simplify + /code-review on fetchWebKey MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit simplify: - fetchWeb derives the key with headers:{} instead of spreading webToHttpRequest — parseRequest reads only the path, so the second header materialization per request is removed. - trim the editorial paragraph duplicated from the README out of the fetchWebKey JSDoc. code-review: - docs claimed the Request's "conditional/Range" headers are honored, but fetchKey forwards only Range (no If-None-Match/If-Modified-Since anywhere on the fetch path). Correct the JSDoc + README to "Range" only. - rewrite the README example to a non-URL key source: the previous one derived the key from Hono's c.req.path, which is already percent-decoded, so it was NOT verbatim and defeated the feature's own point. - fetchWebKey now rejects a null-byte key with the typed InvalidRequest, matching the guard parseRequest applies on every other entry point (was: verbatim key -> generic 500). Adds a test. Co-Authored-By: Claude Opus 4.8 --- README.md | 40 ++++++++++++++++++++++------------------ src/index.ts | 28 ++++++++++++++++------------ test/fetch-web.test.ts | 8 ++++++++ 3 files changed, 46 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 3f6e84d..207f9dd 100644 --- a/README.md +++ b/README.md @@ -341,24 +341,28 @@ return new Response(Readable.toWeb(stream) as ReadableStream, { status, headers #### Custom URL → key mapping (`fetchWebKey`) -When the URL path is not the S3 key — a bucket mounted under `/files/*`, or -keys living under a prefix — use **`proxy.fetchWebKey(key, request)`**. The -caller names the key; the `Request` still supplies the method and -conditional/`Range` headers, and you get the same `Response` with the same HTTP -translation as `fetchWeb`: +When the S3 key doesn't come straight from the request URL — a lookup table, a +database id, a key you've already resolved — use +**`proxy.fetchWebKey(key, request)`**. The caller names the key; the `Request` +still supplies the method and `Range` header, and you get the same `Response` +with the same HTTP translation as `fetchWeb`: ```typescript -// Serve s3://my-bucket/assets/ under the /files/* route. -app.on(['GET', 'HEAD'], '/files/*', (c) => { - const key = `assets/${c.req.path.slice('/files/'.length)}`; +// The key comes from your own mapping, not the URL path. +app.get('/download/:id', async (c) => { + const key = await resolveS3Key(c.req.param('id')); // → the exact S3 object key return proxy.fetchWebKey(key, c.req.raw); }); ``` -This avoids round-tripping the key through URL encode/decode — the subtle bug -where a key containing `%`, `+`, spaces, or other reserved characters serves the -wrong object (or 404s) unless your encoder and s3proxy's decoder agree exactly. -The key is used verbatim. +Because you pass the key directly it is used **verbatim** — s3proxy never +re-parses it out of a URL. That sidesteps the subtle bug where a key containing +`%`, `+`, spaces, or other reserved characters serves the wrong object (or 404s) +because a URL-encode step and s3proxy's decode step disagree. (If you *do* +derive the key from the URL path, you still own decoding it correctly — note +that some frameworks pre-decode their path accessor, e.g. Hono's `c.req.path`; +read the raw URL when the exact bytes matter. For the identity case, just use +`fetchWeb`.) > If you don't need an HTTP `Response` at all (backend processing, ETL, an API > shaping its own headers), use the AWS SDK directly — s3proxy adds nothing @@ -534,12 +538,12 @@ dependency. ##### `await proxy.fetchWebKey(key: string, request: Request): Promise` Key-addressed sibling of `fetchWeb`: the caller supplies the S3 `key` and the -`Request` contributes only the method and conditional/`Range` headers — the URL -is not consulted for key selection. Same HTTP translation and typed-error -behavior as `fetchWeb`. Use it when your URL → key mapping is not identity (a -prefix, a mount point); it avoids the URL encode/decode round-trip that corrupts -keys containing `%`, `+`, spaces, or other reserved characters. The `key` is -used verbatim. +`Request` contributes only the method and `Range` header — the URL is not +consulted for key selection. Same HTTP translation and typed-error behavior as +`fetchWeb`. Use it when the key doesn't come from the URL path (a lookup, a +prefix, a mount point); the `key` is used verbatim, avoiding the URL +encode/decode round-trip that corrupts keys containing `%`, `+`, spaces, or +other reserved characters. ##### `await proxy.healthCheck(): Promise` Verify bucket connectivity. Resolves on success, throws a typed diff --git a/src/index.ts b/src/index.ts index 3093db0..48ec0ab 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,7 @@ import { EventEmitter } from 'node:events'; import { pipeline, Readable } from 'node:stream'; import { GetObjectCommand, HeadBucketCommand, HeadObjectCommand } from '@aws-sdk/client-s3'; -import { S3Forbidden, S3NotFound, S3ProxyError } from './errors.js'; +import { InvalidRequest, S3Forbidden, S3NotFound, S3ProxyError } from './errors.js'; import { mapHeaderToParam, parseRequest } from './request-parser.js'; import { S3Gateway } from './s3-gateway.js'; import type { @@ -103,11 +103,13 @@ export class S3Proxy extends EventEmitter { */ public async fetchWeb(request: Request): Promise { // URL picks the key: derive it from the request path (percent-decoded by - // parseRequest), then hand off to the key-addressed seam. The query string - // plays no role in key selection. + // parseRequest), then hand off to the key-addressed seam. Key derivation + // reads only the path — method, headers, and the query string play no role + // (`url` is required by the HttpRequest type but unused when `path` is set). const key = parseRequest({ - ...this.webToHttpRequest(request), + url: request.url, path: new URL(request.url).pathname, + headers: {}, }).key; return this.fetchWebKey(key, request); } @@ -126,8 +128,8 @@ export class S3Proxy extends EventEmitter { /** * Key-addressed sibling of `fetchWeb`: the *caller* names the S3 key and the - * `Request` contributes only the method (GET/HEAD) and conditional/`Range` - * headers — the request URL plays no role in key selection. Returns the same + * `Request` contributes only the method (GET/HEAD) and the `Range` header — + * the request URL plays no role in key selection. Returns the same * Web `Response` with the same HTTP <-> S3 translation as `fetchWeb` * (`Range` -> 206, missing/denied -> typed 404/403, header mapping, HEAD * semantics, Node -> Web stream), and likewise throws the typed @@ -139,14 +141,16 @@ export class S3Proxy extends EventEmitter { * agree exactly for keys containing `%`, `+`, spaces, or other reserved * characters — or the wrong object is served / a valid key 404s. The key is * used verbatim as the S3 object key. - * - * If you do not need an HTTP `Response` at all (backend processing, ETL, an - * API shaping its own headers), use the AWS SDK directly — s3proxy adds - * nothing there. s3proxy's job is serving S3 objects over HTTP, correctly. */ public async fetchWebKey(key: string, request: Request): Promise { - // fetchKey reads only the method and Range header; the URL carried by - // webToHttpRequest is unused for key selection but keeps the shape valid. + // The key is used verbatim (never decoded); still reject the null byte S3 + // rejects and that every other entry point guards via parseRequest, so the + // typed-error contract (InvalidRequest, not a generic 500) holds here too. + if (key.includes('\0')) { + throw new InvalidRequest('null byte in key'); + } + // fetchKey consumes only the method and Range header; the `url` carried by + // webToHttpRequest is unused for a key-addressed call. const { stream, status, headers } = await this.fetchKey(key, this.webToHttpRequest(request)); // No body for HEAD; otherwise adapt the Node stream to a Web stream. // Backpressure propagates through the wrapper, so nothing buffers. diff --git a/test/fetch-web.test.ts b/test/fetch-web.test.ts index 68a0d85..6eabcec 100644 --- a/test/fetch-web.test.ts +++ b/test/fetch-web.test.ts @@ -136,4 +136,12 @@ describe('proxy.fetchWebKey', () => { InvalidRequest ); }); + + it('rejects a key containing a null byte with the typed InvalidRequest', async () => { + // Matches the guard parseRequest applies on the URL-addressed entry points, + // so the typed-error contract is consistent across the API. + await expect( + proxy.fetchWebKey('reports/bad\0key.txt', new Request(url('/'))) + ).rejects.toBeInstanceOf(InvalidRequest); + }); }); From e7efe614ec10252c934ab92b98a5d75b7ec72e69 Mon Sep 17 00:00:00 2001 From: George Moon Date: Sun, 1 Nov 2026 10:27:52 -0500 Subject: [PATCH 4/4] test: hoist shared fetch-web setup to file scope Both the fetchWeb and fetchWebKey suites used an identical beforeEach/afterEach + `let proxy` block; adding the second suite duplicated it (Codacy flagged the clone). Lift the setup to file scope so it is defined once and applies to both describes. Co-Authored-By: Claude Opus 4.8 --- test/fetch-web.test.ts | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/test/fetch-web.test.ts b/test/fetch-web.test.ts index 6eabcec..2335af8 100644 --- a/test/fetch-web.test.ts +++ b/test/fetch-web.test.ts @@ -6,19 +6,20 @@ import { catchError } from './helpers/http-mocks.js'; const url = (path: string) => `http://localhost${path}`; -describe('proxy.fetchWeb', () => { - let proxy: S3Proxy; +// Shared setup for both suites below: a fresh mocked proxy per test. +let proxy: S3Proxy; - beforeEach(async () => { - setupS3Mocks(); - proxy = new S3Proxy({ bucket: '.test-bucket' }); - await proxy.init(); - }); +beforeEach(async () => { + setupS3Mocks(); + proxy = new S3Proxy({ bucket: '.test-bucket' }); + await proxy.init(); +}); - afterEach(() => { - teardownS3Mocks(); - }); +afterEach(() => { + teardownS3Mocks(); +}); +describe('proxy.fetchWeb', () => { it('returns a 200 Web Response with headers and a streamed body for GET', async () => { const res = await proxy.fetchWeb(new Request(url('/index.html'))); expect(res).toBeInstanceOf(Response); @@ -71,18 +72,6 @@ describe('proxy.fetchWeb', () => { }); describe('proxy.fetchWebKey', () => { - let proxy: S3Proxy; - - beforeEach(async () => { - setupS3Mocks(); - proxy = new S3Proxy({ bucket: '.test-bucket' }); - await proxy.init(); - }); - - afterEach(() => { - teardownS3Mocks(); - }); - it('serves the caller-supplied key, ignoring the request URL', async () => { // URL path is "/" but the key argument selects index.html. const res = await proxy.fetchWebKey('index.html', new Request(url('/')));