Skip to content

Commit

Permalink
Default to unencoded responses (#2834)
Browse files Browse the repository at this point in the history
* Allow defaulting to unencoded responses when proxying client requests that do not specify accept-encoding
* fix content-encoding negotiation
  • Loading branch information
matthieusieben authored Oct 1, 2024
1 parent 2788203 commit 4098d98
Show file tree
Hide file tree
Showing 8 changed files with 274 additions and 108 deletions.
5 changes: 5 additions & 0 deletions .changeset/dirty-foxes-bow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/pds": patch
---

Allow defaulting to unencoded responses when proxying client requests that do not specify accept-encoding
5 changes: 5 additions & 0 deletions .changeset/happy-islands-wash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@atproto/common": patch
---

Allow contentEncoding to be an array for consistency with typing of headers
35 changes: 25 additions & 10 deletions packages/common-web/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,34 @@ export const noUndefinedVals = <T>(
return obj as Record<string, T>
}

/**
* Returns a shallow copy of the object without the specified keys. If the input
* is nullish, it returns the input.
*/
export function omit<
T extends undefined | Record<string, unknown>,
T extends undefined | null | Record<string, unknown>,
K extends keyof NonNullable<T>,
>(obj: T, keys: readonly K[]): T extends undefined ? undefined : Omit<T, K>
>(
object: T,
rejectedKeys: readonly K[],
): T extends undefined ? undefined : T extends null ? null : Omit<T, K>
export function omit(
obj: Record<string, unknown>,
keys: readonly string[],
): undefined | Record<string, unknown> {
if (!obj) return obj

return Object.fromEntries(
Object.entries(obj).filter((entry) => !keys.includes(entry[0])),
)
src: undefined | null | Record<string, unknown>,
rejectedKeys: readonly string[],
): undefined | null | Record<string, unknown> {
// Hot path

if (!src) return src

const dst = {}
const srcKeys = Object.keys(src)
for (let i = 0; i < srcKeys.length; i++) {
const key = srcKeys[i]
if (!rejectedKeys.includes(key)) {
dst[key] = src[key]
}
}
return dst
}

export const jitter = (maxMs: number) => {
Expand Down
25 changes: 19 additions & 6 deletions packages/common/src/streams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,15 @@ export class MaxSizeChecker extends Transform {

export function decodeStream(
stream: Readable,
contentEncoding?: string,
contentEncoding?: string | string[],
): Readable
export function decodeStream(
stream: AsyncIterable<Uint8Array>,
contentEncoding?: string,
contentEncoding?: string | string[],
): AsyncIterable<Uint8Array> | Readable
export function decodeStream(
stream: Readable | AsyncIterable<Uint8Array>,
contentEncoding?: string,
contentEncoding?: string | string[],
): Readable | AsyncIterable<Uint8Array> {
const decoders = createDecoders(contentEncoding)
if (decoders.length === 0) return stream
Expand All @@ -106,14 +106,23 @@ export function decodeStream(
/**
* Create a series of decoding streams based on the content-encoding header. The
* resulting streams should be piped together to decode the content.
*
* @see {@link https://datatracker.ietf.org/doc/html/rfc9110#section-8.4.1}
*/
export function createDecoders(contentEncoding?: string): Duplex[] {
export function createDecoders(contentEncoding?: string | string[]): Duplex[] {
const decoders: Duplex[] = []

if (contentEncoding) {
const encodings = contentEncoding.split(',')
if (contentEncoding?.length) {
const encodings: string[] = Array.isArray(contentEncoding)
? contentEncoding.flatMap(commaSplit)
: contentEncoding.split(',')
for (const encoding of encodings) {
const normalizedEncoding = normalizeEncoding(encoding)

// @NOTE
// > The default (identity) encoding [...] is used only in the
// > Accept-Encoding header, and SHOULD NOT be used in the
// > Content-Encoding header.
if (normalizedEncoding === 'identity') continue

decoders.push(createDecoder(normalizedEncoding))
Expand All @@ -123,6 +132,10 @@ export function createDecoders(contentEncoding?: string): Duplex[] {
return decoders.reverse()
}

function commaSplit(header: string): string[] {
return header.split(',')
}

function normalizeEncoding(encoding: string) {
// https://www.rfc-editor.org/rfc/rfc7231#section-3.1.2.1
// > All content-coding values are case-insensitive...
Expand Down
10 changes: 10 additions & 0 deletions packages/pds/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ export const envToCfg = (env: ServerEnvironment): ServerConfig => {
headersTimeout: env.proxyHeadersTimeout ?? 10e3,
bodyTimeout: env.proxyBodyTimeout ?? 30e3,
maxResponseSize: env.proxyMaxResponseSize ?? 10 * 1024 * 1024, // 10mb
preferCompressed: env.proxyPreferCompressed ?? false,
}

const oauthCfg: ServerConfig['oauth'] = entrywayCfg
Expand Down Expand Up @@ -413,6 +414,15 @@ export type ProxyConfig = {
headersTimeout: number
bodyTimeout: number
maxResponseSize: number

/**
* When proxying requests that might get intercepted (for read-after-write) we
* negotiate the encoding based on the client's preferences. We will however
* use or own weights in order to be able to better control if the PDS will
* need to perform content decoding. This settings allows to prefer compressed
* content over uncompressed one.
*/
preferCompressed: boolean
}

export type OAuthConfig = {
Expand Down
2 changes: 2 additions & 0 deletions packages/pds/src/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ export const readEnv = (): ServerEnvironment => {
proxyHeadersTimeout: envInt('PDS_PROXY_HEADERS_TIMEOUT'),
proxyBodyTimeout: envInt('PDS_PROXY_BODY_TIMEOUT'),
proxyMaxResponseSize: envInt('PDS_PROXY_MAX_RESPONSE_SIZE'),
proxyPreferCompressed: envBool('PDS_PROXY_PREFER_COMPRESSED'),
}
}

Expand Down Expand Up @@ -253,4 +254,5 @@ export type ServerEnvironment = {
proxyHeadersTimeout?: number
proxyBodyTimeout?: number
proxyMaxResponseSize?: number
proxyPreferCompressed?: boolean
}
Loading

0 comments on commit 4098d98

Please sign in to comment.