Default to unencoded responses (bluesky-social#2834)

* Allow defaulting to unencoded responses when proxying client requests that do not specify accept-encoding
* fix content-encoding negotiation

.changeset/dirty-foxes-bow.md

@@ -0,0 +1,5 @@
+---
+"@atproto/pds": patch
+---
+
+Allow defaulting to unencoded responses when proxying client requests that do not specify accept-encoding

.changeset/happy-islands-wash.md

@@ -0,0 +1,5 @@
+---
+"@atproto/common": patch
+---
+
+Allow contentEncoding to be an array for consistency with typing of headers

packages/common-web/src/util.ts

@@ -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) => {

packages/common/src/streams.ts

@@ -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
@@ -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))
@@ -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...

packages/pds/src/config/config.ts

@@ -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
@@ -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 = {

packages/pds/src/config/env.ts

@@ -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'),
   }
 }
 
@@ -253,4 +254,5 @@ export type ServerEnvironment = {
   proxyHeadersTimeout?: number
   proxyBodyTimeout?: number
   proxyMaxResponseSize?: number
+  proxyPreferCompressed?: boolean
 }