diff --git a/src/http/plugins/header-validator.test.ts b/src/http/plugins/header-validator.test.ts index c454c2b1a..d55f8e418 100644 --- a/src/http/plugins/header-validator.test.ts +++ b/src/http/plugins/header-validator.test.ts @@ -30,32 +30,6 @@ describe('header-validator plugin', () => { expect(body.message).toContain('x-test') }) - it('should reject response with carriage return in header value', async () => { - app.get('/test', async (_request, reply) => { - reply.header('x-custom', 'value\rwith\rCR') - return { ok: true } - }) - - const response = await app.inject({ method: 'GET', url: '/test' }) - - expect(response.statusCode).toBe(400) - const body = response.json() - expect(body.error).toBe('Bad Request') - expect(body.message).toContain('Invalid character in response header') - }) - - it('should allow valid header values with TAB character', async () => { - app.get('/test', async (_request, reply) => { - reply.header('x-custom', 'value\twith\ttabs') - return { ok: true } - }) - - const response = await app.inject({ method: 'GET', url: '/test' }) - - expect(response.statusCode).toBe(200) - expect(response.headers['x-custom']).toBe('value\twith\ttabs') - }) - it('should allow normal ASCII header values', async () => { app.get('/test', async (_request, reply) => { reply.header('x-transformations', 'width:100,height:200,resize:cover') diff --git a/src/http/plugins/header-validator.ts b/src/http/plugins/header-validator.ts index 6e7f9e8be..a239a5fbb 100644 --- a/src/http/plugins/header-validator.ts +++ b/src/http/plugins/header-validator.ts @@ -1,15 +1,8 @@ import { ERRORS } from '@internal/errors' +import { hasInvalidHeaderValueChars } from '@internal/http/header' import { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify' import fastifyPlugin from 'fastify-plugin' -/** - * Matches invalid HTTP header characters per RFC 7230 field-vchar specification. - * Valid: TAB (0x09), visible ASCII (0x20-0x7E), obs-text (0x80-0xFF). - * Invalid: control characters (0x00-0x1F except TAB) and DEL (0x7F). - * @see https://tools.ietf.org/html/rfc7230#section-3.2 - */ -const INVALID_HEADER_CHAR_PATTERN = /[^\t\x20-\x7e\x80-\xff]/ - interface HeaderValidatorOptions { excludeUrls?: string[] } @@ -34,11 +27,14 @@ export const headerValidator = (options: HeaderValidatorOptions = {}) => continue } const value = headers[key] - if (typeof value === 'string' && INVALID_HEADER_CHAR_PATTERN.test(value)) { - throw ERRORS.InvalidHeaderChar(key, value) + if (typeof value === 'string') { + if (hasInvalidHeaderValueChars(value)) { + throw ERRORS.InvalidHeaderChar(key, value) + } } else if (Array.isArray(value)) { - for (const item of value) { - if (typeof item === 'string' && INVALID_HEADER_CHAR_PATTERN.test(item)) { + for (let j = 0; j < value.length; j++) { + const item = value[j] + if (typeof item === 'string' && hasInvalidHeaderValueChars(item)) { throw ERRORS.InvalidHeaderChar(key, item) } } diff --git a/src/http/routes/s3/index.ts b/src/http/routes/s3/index.ts index 4b2c0d5f3..1483f452b 100644 --- a/src/http/routes/s3/index.ts +++ b/src/http/routes/s3/index.ts @@ -104,8 +104,9 @@ export default async function routes(fastify: FastifyInstance) { continue } - if (headers[header]) { - reply.header(header, headers[header]) + const value = headers[header] + if (value || (value === '' && header.startsWith('x-amz-meta-'))) { + reply.header(header, value) } } } diff --git a/src/http/routes/s3/router.test.ts b/src/http/routes/s3/router.test.ts index bc823f6ff..cb4f393c4 100644 --- a/src/http/routes/s3/router.test.ts +++ b/src/http/routes/s3/router.test.ts @@ -422,6 +422,52 @@ describe('S3 route handler matching', () => { expect(setAttribute).toHaveBeenCalledWith('http.operation', 'storage.s3.bucket.list') }) + + it('forwards empty S3 metadata response headers', async () => { + const findObject = vi.fn().mockResolvedValue({ + created_at: '2026-06-25T00:00:00.000Z', + metadata: { + eTag: '"etag"', + mimetype: 'text/plain', + size: '0', + }, + updated_at: '2026-06-25T00:00:00.000Z', + user_metadata: { + empty: '', + }, + }) + + await withMockedS3App( + async (app) => { + const response = await app.inject({ + method: 'HEAD', + url: '/bucket/object.txt', + }) + + expect(response.statusCode).toBe(200) + expect(response.headers['x-amz-meta-empty']).toBe('') + expect(response.headers.expires).toBeUndefined() + expect(response.headers['cache-control']).toBeUndefined() + }, + { + configureRequest: (request) => { + Object.assign(request, { + owner: 'owner-id', + signals: { + body: new AbortController(), + response: new AbortController(), + }, + storage: { + from: vi.fn(() => ({ + findObject, + })), + }, + tenantId: 'tenant-id', + }) + }, + } + ) + }) }) describe('S3 router type matching', () => { @@ -586,6 +632,18 @@ describe('S3ProtocolHandler.parseMetadataHeaders', () => { }) }) + it('keeps empty string metadata values', () => { + const handler = createHandler() + + expect( + handler.parseMetadataHeaders({ + 'x-amz-meta-empty': '', + }) + ).toEqual({ + empty: '', + }) + }) + it('returns undefined when there are no metadata headers', () => { const handler = createHandler() diff --git a/src/internal/http/header.test.ts b/src/internal/http/header.test.ts new file mode 100644 index 000000000..f5a7cb8b4 --- /dev/null +++ b/src/internal/http/header.test.ts @@ -0,0 +1,95 @@ +import { + hasInvalidHeaderValueChars, + isValidHeader, + MAX_HEADER_NAME_LENGTH, + MAX_HEADER_VALUE_LENGTH, +} from './header' + +describe('hasInvalidHeaderValueChars', () => { + it.each([ + ['empty value', ''], + ['visible ASCII', 'width:100,height:200,resize:cover'], + ['horizontal tab', 'value\twith\ttabs'], + ['obs-text upper byte range', `value${String.fromCharCode(0x80)}${String.fromCharCode(0xff)}`], + ])('allows %s', (_name, value) => { + expect(hasInvalidHeaderValueChars(value)).toBe(false) + }) + + it.each([ + ['NUL', '\x00'], + ['unit separator', '\x1f'], + ['line feed', '\n'], + ['carriage return', '\r'], + ['DEL', '\x7f'], + ['code point above one byte', '\u0100'], + ])('rejects %s', (_name, value) => { + expect(hasInvalidHeaderValueChars(`value${value}`)).toBe(true) + }) +}) + +describe('isValidHeader', () => { + it('accepts a typical header name and value', () => { + expect(isValidHeader('content-type', 'application/json')).toBe(true) + }) + + it('accepts an empty header value', () => { + expect(isValidHeader('x-custom', '')).toBe(true) + }) + + it('accepts all token chars permitted by RFC7230 section 3.2.6', () => { + expect(isValidHeader("!#$%&'*+-.^_`|~09AZaz", 'v')).toBe(true) + }) + + it('rejects header names containing characters outside the token set', () => { + expect(isValidHeader('bad name', 'v')).toBe(false) + expect(isValidHeader('bad:name', 'v')).toBe(false) + expect(isValidHeader('bad(name)', 'v')).toBe(false) + expect(isValidHeader('badåname', 'v')).toBe(false) + }) + + it('rejects an empty header name', () => { + expect(isValidHeader('', 'v')).toBe(false) + }) + + it('rejects header names exceeding the max length', () => { + const oversizedName = 'a'.repeat(MAX_HEADER_NAME_LENGTH + 1) + expect(isValidHeader(oversizedName, 'value')).toBe(false) + }) + + it('accepts header names exactly at the max length', () => { + const maxName = 'a'.repeat(MAX_HEADER_NAME_LENGTH) + expect(isValidHeader(maxName, 'value')).toBe(true) + }) + + it('rejects header values containing control characters', () => { + expect(isValidHeader('x-custom', 'bad\x00value')).toBe(false) + expect(isValidHeader('x-custom', 'bad\nvalue')).toBe(false) + }) + + it('rejects header values containing CRLF', () => { + expect(isValidHeader('x-custom', 'innocent\r\nX-Injected: 1')).toBe(false) + }) + + it('rejects header values exceeding the max byte length', () => { + const oversizedValue = 'a'.repeat(MAX_HEADER_VALUE_LENGTH + 1) + expect(isValidHeader('x-custom', oversizedValue)).toBe(false) + }) + + it('accepts header values exactly at the max byte length', () => { + const maxValue = 'a'.repeat(MAX_HEADER_VALUE_LENGTH) + expect(isValidHeader('x-custom', maxValue)).toBe(true) + }) + + it('counts obs-text values by UTF-8 byte length', () => { + expect(isValidHeader('x-custom', String.fromCharCode(0x80).repeat(4096))).toBe(true) + expect(isValidHeader('x-custom', String.fromCharCode(0x80).repeat(4097))).toBe(false) + }) + + it('accepts an array of values when all are valid', () => { + expect(isValidHeader('x-custom', ['one', 'two', 'three'])).toBe(true) + }) + + it('rejects an array of values when any are invalid', () => { + expect(isValidHeader('x-custom', ['ok', 'bad\x00value'])).toBe(false) + }) +}) diff --git a/src/internal/http/header.ts b/src/internal/http/header.ts new file mode 100644 index 000000000..eccf83c9a --- /dev/null +++ b/src/internal/http/header.ts @@ -0,0 +1,91 @@ +export const MAX_HEADER_NAME_LENGTH = 1024 * 8 // 8KB +export const MAX_HEADER_VALUE_LENGTH = 1024 * 8 // 8KB + +/** + * Checks whether a character code is valid in an HTTP token per RFC 7230. + * Header names are tokens: alphanumeric plus !#$%&'*+-.^_`|~. + * @see https://tools.ietf.org/html/rfc7230#section-3.2.6 + */ +const isHttpTokenCharCode = (c: number): boolean => + (c >= 0x30 && c <= 0x39) || + (c >= 0x41 && c <= 0x5a) || + (c >= 0x61 && c <= 0x7a) || + c === 0x21 || + (c >= 0x23 && c <= 0x27) || + c === 0x2a || + c === 0x2b || + c === 0x2d || + c === 0x2e || + c === 0x5e || + c === 0x5f || + c === 0x60 || + c === 0x7c || + c === 0x7e + +/** + * Checks if a string contains invalid HTTP header characters per RFC 7230. + * Valid: TAB (0x09), visible ASCII (0x20-0x7E), obs-text (0x80-0xFF). + * Invalid: control characters (0x00-0x1F except TAB), DEL (0x7F), and >0xFF. + * Uses charCodeAt for lower overhead than regex on short header values. + * @see https://tools.ietf.org/html/rfc7230#section-3.2 + */ +const isInvalidHeaderValueCharCode = (c: number): boolean => + c > 0xff || (c < 0x20 && c !== 0x09) || c === 0x7f + +export function hasInvalidHeaderValueChars(value: string): boolean { + for (let i = 0; i < value.length; i++) { + if (isInvalidHeaderValueCharCode(value.charCodeAt(i))) { + return true + } + } + return false +} + +export function isValidHeaderName(name: string): boolean { + if (name.length === 0 || name.length > MAX_HEADER_NAME_LENGTH) { + return false + } + + for (let i = 0; i < name.length; i++) { + if (!isHttpTokenCharCode(name.charCodeAt(i))) { + return false + } + } + + return true +} + +export function isValidHeaderValue(value: string): boolean { + let byteLength = 0 + + for (let i = 0; i < value.length; i++) { + const c = value.charCodeAt(i) + if (isInvalidHeaderValueCharCode(c)) { + return false + } + + byteLength += c < 0x80 ? 1 : 2 + if (byteLength > MAX_HEADER_VALUE_LENGTH) { + return false + } + } + + return true +} + +export function isValidHeader(name: string, value: string | string[]): boolean { + if (!isValidHeaderName(name)) { + return false + } + + if (Array.isArray(value)) { + for (let i = 0; i < value.length; i++) { + if (!isValidHeaderValue(value[i])) { + return false + } + } + return true + } + + return isValidHeaderValue(value) +} diff --git a/src/internal/http/index.ts b/src/internal/http/index.ts index c9209a598..91c31834a 100644 --- a/src/internal/http/index.ts +++ b/src/internal/http/index.ts @@ -1 +1,2 @@ export * from './agent' +export * from './header' diff --git a/src/storage/protocols/s3/s3-handler.test.ts b/src/storage/protocols/s3/s3-handler.test.ts index 84d558fcc..a35e5fb35 100644 --- a/src/storage/protocols/s3/s3-handler.test.ts +++ b/src/storage/protocols/s3/s3-handler.test.ts @@ -1,77 +1,102 @@ -import { vi } from 'vitest' +import { MAX_HEADER_NAME_LENGTH } from '@internal/http/header' +import { S3ProtocolHandler } from '@storage/protocols/s3/s3-handler' -// fs-xattr is pulled in transitively through the file backend; it has no Windows build. -vi.mock('fs-xattr', () => ({ - set: vi.fn(() => Promise.resolve()), - get: vi.fn(() => Promise.resolve(undefined)), -})) - -import { isValidHeader, S3ProtocolHandler } from '@storage/protocols/s3/s3-handler' - -// Mirror the constants in s3-handler.ts -const MAX_HEADER_NAME_LENGTH = 1024 * 8 -const MAX_HEADER_VALUE_LENGTH = 1024 * 8 - -describe('isValidHeader', () => { - it('accepts a typical header name and value', () => { - expect(isValidHeader('content-type', 'application/json')).toBe(true) - }) - - it('accepts all token chars permitted by RFC7230 §3.2.6', () => { - expect(isValidHeader("!#$%&'*+-.^_`|~09AZaz", 'v')).toBe(true) - }) - - it('rejects header names containing characters outside the token set', () => { - expect(isValidHeader('bad name', 'v')).toBe(false) - expect(isValidHeader('bad:name', 'v')).toBe(false) - expect(isValidHeader('bad(name)', 'v')).toBe(false) - }) - - it('rejects an empty header name', () => { - expect(isValidHeader('', 'v')).toBe(false) - }) - - it('rejects header names exceeding the max byte length', () => { - const oversizedName = 'a'.repeat(MAX_HEADER_NAME_LENGTH + 1) - expect(isValidHeader(oversizedName, 'value')).toBe(false) - }) +describe('S3ProtocolHandler.dbHeadObject', () => { + it('emits empty user metadata values as valid S3 metadata headers', async () => { + const findObject = vi.fn().mockResolvedValue({ + created_at: '2026-06-25T00:00:00.000Z', + metadata: { + eTag: '"etag"', + mimetype: 'text/plain', + size: '0', + }, + updated_at: '2026-06-25T00:00:00.000Z', + user_metadata: { + color: 'blue', + empty: '', + }, + }) + const storage = { + from: vi.fn(() => ({ + findObject, + })), + } + const handler = new S3ProtocolHandler(storage as never, 'tenant-id') - it('rejects oversized names even when all characters are otherwise valid', () => { - // Long + regex-matching still has to fail: the length check must not be bypassed. - const oversizedValid = 'a'.repeat(MAX_HEADER_NAME_LENGTH + 100) - expect(isValidHeader(oversizedValid, 'ok')).toBe(false) - }) + const response = await handler.dbHeadObject({ + Bucket: 'bucket', + Key: 'object.txt', + }) - it('accepts header names exactly at the max byte length', () => { - const maxName = 'a'.repeat(MAX_HEADER_NAME_LENGTH) - expect(isValidHeader(maxName, 'value')).toBe(true) + expect(response.headers).toMatchObject({ + 'x-amz-meta-color': 'blue', + 'x-amz-meta-empty': '', + }) + expect(response.headers).not.toHaveProperty('x-amz-missing-meta') }) - it('rejects header values containing control characters', () => { - expect(isValidHeader('x-custom', 'bad\x00value')).toBe(false) - expect(isValidHeader('x-custom', 'bad\nvalue')).toBe(false) - }) + it('counts metadata as missing when the emitted S3 metadata header name is too long', async () => { + const prefix = 'x-amz-meta-' + const key = 'a'.repeat(MAX_HEADER_NAME_LENGTH - prefix.length + 1) + const findObject = vi.fn().mockResolvedValue({ + created_at: '2026-06-25T00:00:00.000Z', + metadata: { + eTag: '"etag"', + mimetype: 'text/plain', + size: '0', + }, + updated_at: '2026-06-25T00:00:00.000Z', + user_metadata: { + [key]: 'value', + }, + }) + const storage = { + from: vi.fn(() => ({ + findObject, + })), + } + const handler = new S3ProtocolHandler(storage as never, 'tenant-id') - it('rejects header values containing CRLF (header injection)', () => { - expect(isValidHeader('x-custom', 'innocent\r\nX-Injected: 1')).toBe(false) - }) + const response = await handler.dbHeadObject({ + Bucket: 'bucket', + Key: 'object.txt', + }) - it('rejects header values exceeding the max byte length', () => { - const oversizedValue = 'a'.repeat(MAX_HEADER_VALUE_LENGTH + 1) - expect(isValidHeader('x-custom', oversizedValue)).toBe(false) + expect(response.headers).toHaveProperty('x-amz-missing-meta', 1) + expect(response.headers).not.toHaveProperty(prefix + key) }) - it('accepts header values exactly at the max byte length', () => { - const maxValue = 'a'.repeat(MAX_HEADER_VALUE_LENGTH) - expect(isValidHeader('x-custom', maxValue)).toBe(true) - }) + it('counts empty user metadata keys as missing', async () => { + const findObject = vi.fn().mockResolvedValue({ + created_at: '2026-06-25T00:00:00.000Z', + metadata: { + eTag: '"etag"', + mimetype: 'text/plain', + size: '0', + }, + updated_at: '2026-06-25T00:00:00.000Z', + user_metadata: { + '': 'value', + color: 'blue', + }, + }) + const storage = { + from: vi.fn(() => ({ + findObject, + })), + } + const handler = new S3ProtocolHandler(storage as never, 'tenant-id') - it('accepts an array of values when all are valid', () => { - expect(isValidHeader('x-custom', ['one', 'two', 'three'])).toBe(true) - }) + const response = await handler.dbHeadObject({ + Bucket: 'bucket', + Key: 'object.txt', + }) - it('rejects an array of values when any are invalid', () => { - expect(isValidHeader('x-custom', ['ok', 'bad\x00value'])).toBe(false) + expect(response.headers).toMatchObject({ + 'x-amz-meta-color': 'blue', + 'x-amz-missing-meta': 1, + }) + expect(response.headers).not.toHaveProperty('x-amz-meta-') }) }) diff --git a/src/storage/protocols/s3/s3-handler.ts b/src/storage/protocols/s3/s3-handler.ts index a96cd1991..cbba5ab05 100644 --- a/src/storage/protocols/s3/s3-handler.ts +++ b/src/storage/protocols/s3/s3-handler.ts @@ -19,6 +19,7 @@ import { } from '@aws-sdk/client-s3' import { decrypt, encrypt } from '@internal/auth' import { ERRORS, ErrorCode, isStorageError } from '@internal/errors' +import { isValidHeader } from '@internal/http/header' import { logger, logSchema } from '@internal/monitoring' import { PassThrough, Readable } from 'stream' import stream from 'stream/promises' @@ -856,7 +857,7 @@ export class S3ProtocolHandler { let metadataHeaders: Record = {} if (object.user_metadata) { - metadataHeaders = toAwsMeatadataHeaders(object.user_metadata) + metadataHeaders = toAwsMetadataHeaders(object.user_metadata) } return { @@ -943,7 +944,7 @@ export class S3ProtocolHandler { let metadataHeaders: Record = {} if (userMetadata) { - metadataHeaders = toAwsMeatadataHeaders(userMetadata) + metadataHeaders = toAwsMetadataHeaders(userMetadata) } const headers: Record = { @@ -1450,28 +1451,7 @@ export class S3ProtocolHandler { } } -const MAX_HEADER_NAME_LENGTH = 1024 * 8 // 8KB, per RFC7230 §3.2.6 -const MAX_HEADER_VALUE_LENGTH = 1024 * 8 // 8KB, per RFC7230 §3.2.4 - -// Allowed header‐name chars per RFC7230 §3.2.6 “token” -// (note that the backtick ` is escaped) -const HEADER_NAME_RE = /^[!#$%&'*+\-.\^_`|~0-9A-Za-z]+$/ - -// Allowed header‐value chars per RFC7230 §3.2.4 -// (horizontal tab, space–tilde, and 0x80–0xFF) -const HEADER_VALUE_RE = /^[\t\x20-\x7e\x80-\xff]+$/ - -export function isValidHeader(name: string, value: string | string[]): boolean { - if (Buffer.from(`${name}`).byteLength > MAX_HEADER_NAME_LENGTH || !HEADER_NAME_RE.test(name)) { - return false - } - const values = Array.isArray(value) ? value : [value] - return values.every( - (v) => Buffer.from(`${v}`).byteLength <= MAX_HEADER_VALUE_LENGTH && HEADER_VALUE_RE.test(v) - ) -} - -function toAwsMeatadataHeaders(records: Record) { +function toAwsMetadataHeaders(records: Record) { const metadataHeaders: Record = {} let missingCount = 0 @@ -1480,9 +1460,15 @@ function toAwsMeatadataHeaders(records: Record) { continue } + if (key.length === 0) { + missingCount++ + continue + } + const value = records[key] - if (value && typeof value === 'string' && isUSASCII(value) && isValidHeader(key, value)) { - metadataHeaders['x-amz-meta-' + key.toLowerCase()] = value + const headerName = 'x-amz-meta-' + key.toLowerCase() + if (typeof value === 'string' && isUSASCII(value) && isValidHeader(headerName, value)) { + metadataHeaders[headerName] = value } else { missingCount++ }