Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion examples/express-basic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
Author: George Moon <george.moon@gmail.com>
*/

import { pipeline } from 'node:stream';
import { XmlNode, XmlText } from '@aws-sdk/xml-builder';
import bodyParser from 'body-parser';
import express, { type Request, type Response } from 'express';
Expand Down Expand Up @@ -108,7 +109,11 @@ app
try {
const { stream, status, headers } = await proxy.fetch(req as unknown as HttpRequest);
res.writeHead(status, headers);
stream.on('error', (err: ErrorWithDetails) => handleError(req, res, err)).pipe(res);
// pipeline (not .pipe) destroys the S3 source stream on client disconnect
// or write error, so the underlying S3 socket isn't leaked.
pipeline(stream, res, (err) => {
if (err) handleError(req, res, err as ErrorWithDetails);
});
} catch (error) {
handleError(req, res, error as ErrorWithDetails);
}
Expand Down
17 changes: 6 additions & 11 deletions examples/fastify-basic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* v4 proxy.fetch() API: pure data fetch, frameworks own the response.
*/

import { pipeline } from 'node:stream';
import { XmlNode, XmlText } from '@aws-sdk/xml-builder';
import Fastify from 'fastify';
import { S3Proxy } from '../src/index.js';
Expand Down Expand Up @@ -60,18 +61,12 @@ fastify.head('/*', async (request, reply) => {
fastify.get('/*', async (request, reply) => {
const { stream, status, headers } = await proxy.fetch(request.raw as unknown as HttpRequest);
reply.raw.writeHead(status, headers);
stream.on('error', (err: S3Error) => {
const errorXml = createErrorXml(err, request.url);
if (!reply.raw.headersSent) {
reply
.status(err.statusCode || 500)
.type('application/xml')
.send(errorXml);
} else {
reply.raw.end();
}
// pipeline (not .pipe) destroys the S3 source stream if the client disconnects
// mid-transfer, so the underlying S3 socket isn't leaked. Headers are already
// sent by writeHead above, so on a stream error we can only end the response.
pipeline(stream, reply.raw, (err) => {
if (err && !reply.raw.writableEnded) reply.raw.end();
});
stream.pipe(reply.raw);
return reply.hijack();
});

Expand Down
7 changes: 6 additions & 1 deletion examples/http.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createServer } from 'node:http';
import { pipeline } from 'node:stream';
import { S3Proxy } from '../src/index.js';
import type { HttpRequest } from '../src/types.js';

Expand All @@ -18,7 +19,11 @@ const server = createServer(async (req, res) => {
try {
const { stream, status, headers } = await proxy.fetch(req as unknown as HttpRequest);
res.writeHead(status, headers);
stream.on('error', () => res.end()).pipe(res);
// pipeline (not .pipe) destroys the S3 source stream if the client
// disconnects mid-transfer, so the underlying S3 socket isn't leaked.
pipeline(stream, res, (err) => {
if (err && !res.writableEnded) res.end();
});
} catch (error) {
const statusCode =
typeof error === 'object' &&
Expand Down
12 changes: 9 additions & 3 deletions src/s3-gateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
S3Client,
S3ServiceException,
} from '@aws-sdk/client-s3';
import { S3Forbidden, S3InvalidRange, S3NotFound, type S3ProxyError } from './errors.js';
import { S3Forbidden, S3InvalidRange, S3NotFound, S3ProxyError } from './errors.js';
import type { S3FetchResponse, S3ProxyOptions } from './types.js';
import { UserException } from './UserException.js';

Expand Down Expand Up @@ -55,15 +55,21 @@ function outputToHeaders(output: SupportedOutput): Record<string, string> {
return h;
}

function mapError(e: unknown, target: string): S3ProxyError | never {
function mapError(e: unknown, target: string): S3ProxyError {
if (e instanceof NoSuchKey || e instanceof NoSuchBucket) {
return new S3NotFound(target, { cause: e });
}
if (e instanceof S3ServiceException) {
if (e.name === 'AccessDenied') return new S3Forbidden(target, { cause: e });
if (e.name === 'InvalidRange') return new S3InvalidRange(target, { cause: e });
}
throw e;
// Anything else — an unmapped S3ServiceException (e.g. PermanentRedirect,
// SlowDown, InternalError), a network error, or an unexpected throw — is
// wrapped in a generic 500. Raw AWS details (bucket region/endpoint,
// requestId, internal messages) must never reach the client; the original
// is preserved on `cause` for server-side logging only. Consumers that log
// errors should log `err.cause`, never surface it in a response body.
return new S3ProxyError(`S3 request failed: ${target}`, 500, { cause: e });
}

/**
Expand Down
19 changes: 14 additions & 5 deletions test/fetch.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { GetObjectCommand, S3ServiceException } from '@aws-sdk/client-s3';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { S3Forbidden, S3InvalidRange, S3NotFound, S3Proxy } from '../src/index.js';
import { S3Forbidden, S3InvalidRange, S3NotFound, S3Proxy, S3ProxyError } from '../src/index.js';
import { s3Mock, setupS3Mocks, teardownS3Mocks } from './helpers/aws-mock.js';
import { catchError, makeReq, readAll } from './helpers/http-mocks.js';

Expand Down Expand Up @@ -75,20 +75,29 @@ describe('proxy.fetch', () => {
expect((err as S3InvalidRange).cause).toBe(invalidRange);
});

it('rethrows non-AWS errors unchanged', async () => {
it('wraps non-AWS errors in a sanitized 500 (original preserved on cause)', async () => {
// Unmapped errors (network failures, unexpected AWS exceptions like
// PermanentRedirect/SlowDown) must not leak their raw message to the client.
const networkError = new Error('Network error');
s3Mock.on(GetObjectCommand, { Bucket: '.test-bucket', Key: 'flaky.txt' }).rejects(networkError);
await expect(proxy.fetch(makeReq('/flaky.txt'))).rejects.toThrow('Network error');
const err = await catchError(proxy.fetch(makeReq('/flaky.txt')));
expect(err).toBeInstanceOf(S3ProxyError);
expect((err as S3ProxyError).statusCode).toBe(500);
expect((err as Error).message).not.toContain('Network error');
expect((err as S3ProxyError).cause).toBe(networkError);
});

it('throws for unrecognized body type', async () => {
it('wraps an unrecognized body type in a sanitized 500 (original on cause)', async () => {
s3Mock.on(GetObjectCommand, { Bucket: '.test-bucket', Key: 'weird.txt' }).resolves({
Body: { invalid: 'body' } as never,
ContentLength: 100,
ContentType: 'text/plain',
$metadata: { httpStatusCode: 200, requestId: 'mock' },
});
await expect(proxy.fetch(makeReq('/weird.txt'))).rejects.toThrow('unrecognized type');
const err = await catchError(proxy.fetch(makeReq('/weird.txt')));
expect(err).toBeInstanceOf(S3ProxyError);
expect((err as S3ProxyError).statusCode).toBe(500);
expect(((err as S3ProxyError).cause as Error).message).toBe('unrecognized type');
});

it('does not write to a response (pure)', async () => {
Expand Down
23 changes: 17 additions & 6 deletions test/s3proxy.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { HeadBucketCommand } from '@aws-sdk/client-s3';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { S3NotFound, S3Proxy, UserException } from '../src/index.js';
import { S3NotFound, S3Proxy, S3ProxyError, UserException } from '../src/index.js';
import type { S3ProxyConfig } from '../src/types.js';
import { s3Mock, setupS3Mocks, teardownS3Mocks } from './helpers/aws-mock.js';
import { catchError } from './helpers/http-mocks.js';

describe('S3Proxy', () => {
beforeEach(() => {
Expand Down Expand Up @@ -72,8 +73,14 @@ describe('S3Proxy', () => {
const errorSpy = vi.fn();
proxy.on('error', errorSpy);

await expect(proxy.init()).rejects.toThrow('Init failed');
expect(errorSpy).toHaveBeenCalledWith(initError);
// init() rejects with a sanitized S3ProxyError (raw message not exposed);
// the original is preserved on `cause` and the same error is emitted.
const err = await catchError(proxy.init());
expect(err).toBeInstanceOf(S3ProxyError);
expect((err as S3ProxyError).statusCode).toBe(500);
expect((err as Error).message).not.toContain('Init failed');
expect((err as S3ProxyError).cause).toBe(initError);
expect(errorSpy).toHaveBeenCalledWith(err);
});
});

Expand All @@ -99,10 +106,14 @@ describe('S3Proxy', () => {
});

describe('verifyOnInit', () => {
it('defaults to true: rejects when the bucket is unreachable', async () => {
it('defaults to true: rejects with a sanitized error when the bucket is unreachable', async () => {
const proxy = new S3Proxy({ bucket: '.test-bucket' });
s3Mock.on(HeadBucketCommand).rejectsOnce(new Error('unreachable'));
await expect(proxy.init()).rejects.toThrow('unreachable');
const cause = new Error('unreachable');
s3Mock.on(HeadBucketCommand).rejectsOnce(cause);
const err = await catchError(proxy.init());
expect(err).toBeInstanceOf(S3ProxyError);
expect((err as Error).message).not.toContain('unreachable');
expect((err as S3ProxyError).cause).toBe(cause);
});

it('false: init() resolves without sending a HeadBucket', async () => {
Expand Down
Loading