feat: update vary to use res.appendHeader

packages/vary/src/index.ts

@@ -1,4 +1,9 @@
-import type { ServerResponse as Response } from 'node:http'
+import type { OutgoingMessage } from 'node:http'
+
+type HasHeaders = Pick<
+  OutgoingMessage,
+  'getHeader' | 'getHeaders' | 'setHeader' | 'appendHeader' | 'getHeaderNames' | 'hasHeader' | 'removeHeader'
+>
 
 /**
  * RegExp to match field-name in RFC 7230 sec 3.2
@@ -10,9 +15,15 @@ import type { ServerResponse as Response } from 'node:http'
  *               / DIGIT / ALPHA
  *               ; any VCHAR, except delimiters
  */
-
 const FIELD_NAME_REGEXP = /^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$/
 
+function getVaryHeader(res: HasHeaders): string[] {
+  const vary = res.getHeader('vary')
+  if (Array.isArray(vary)) return vary
+  if (typeof vary === 'string') return parse(vary.toLowerCase())
+  return []
+}
+
 function parse(header: string) {
   let end = 0
   const list: string[] = []
@@ -42,51 +53,39 @@ function parse(header: string) {
   return list
 }
 
-export function append(header: string, field: string | string[]) {
+/**
+ * Mark that a request is varied on a header field.
+ */
+export function vary(res: HasHeaders, field: string | string[]) {
+  // get existing header
+  const val = getVaryHeader(res)
+  const alreadySetHeaderNameLookup = new Set<string>(val)
+
   // get fields array
-  const fields = !Array.isArray(field) ? parse(String(field)) : field
+  const fields = Array.isArray(field) ? field : parse(field)
 
-  // assert on invalid field names
+  // ensure field names are valid
   for (const field of fields) {
     if (!FIELD_NAME_REGEXP.test(field)) throw new TypeError('field argument contains an invalid header name')
   }
 
-  // existing, unspecified vary
-  if (header === '*') {
-    return header
-  }
-
-  // enumerate current values
-  let val = header
-  const vals = parse(header.toLowerCase())
-
-  // unspecified vary
-  if (fields.indexOf('*') !== -1 || vals.indexOf('*') !== -1) {
-    return '*'
+  if (alreadySetHeaderNameLookup.has('*')) {
+    if (val.length > 1) res.setHeader('vary', ['*'])
+    return
   }
 
-  for (const field of fields) {
-    const fld = field.toLowerCase()
-
-    // append value (case-preserving)
-    if (vals.indexOf(fld) === -1) {
-      vals.push(fld)
-      val = val ? `${val}, ${field}` : field
+  const unsetHeaderNamesToSet: string[] = []
+  for (const headerName of fields) {
+    if (headerName === '*') {
+      res.setHeader('vary', ['*'])
+      return
     }
-  }
 
-  return val
-}
-/**
- * Mark that a request is varied on a header field.
- */
-export function vary(res: Response, field: string | string[]) {
-  // get existing header
-  let val = res.getHeader('Vary') || ''
-  const header = Array.isArray(val) ? val.join(', ') : String(val)
-
-  // set new header
-  if ((val = append(header, field))) {
-    res.setHeader('Vary', val)
+    const lowerCaseHeaderName = headerName.toLowerCase()
+    if (alreadySetHeaderNameLookup.has(lowerCaseHeaderName)) continue
+    unsetHeaderNamesToSet.push(headerName)
+    alreadySetHeaderNameLookup.add(lowerCaseHeaderName)
   }
+
+  res.appendHeader('vary', unsetHeaderNamesToSet)
 }

tests/modules/vary.test.ts

@@ -1,137 +1,193 @@
+import { IncomingMessage, ServerResponse } from 'node:http'
+import type { Socket } from 'node:net'
 import { describe, expect, it } from 'vitest'
 
-import { append } from '@/packages/vary/src'
+import { vary } from '@/packages/vary/src'
+
+function getMockResponse(initialVaryHeader?: string | string[]) {
+  const req = new IncomingMessage(undefined as unknown as Socket)
+  const res = new ServerResponse(req)
+  if (initialVaryHeader != null) res.setHeader('vary', initialVaryHeader)
+  return res
+}
 
 describe('field', () => {
   it('should accept string', () => {
+    const res = getMockResponse()
     expect(() => {
-      append('', 'foo')
+      vary(res, 'foo')
     }).not.toThrow()
   })
 
   it('should accept string that is Vary header', () => {
+    const res = getMockResponse()
     expect(() => {
-      append('', 'foo, bar')
+      vary(res, 'foo, bar')
     }).not.toThrow()
   })
 
   it('should accept array of string', () => {
+    const res = getMockResponse()
     expect(() => {
-      append('', ['foo', 'bar'])
+      vary(res, ['foo', 'bar'])
     }).not.toThrow()
   })
 
   it('should not allow separator ":"', () => {
+    const res = getMockResponse()
     expect(() => {
-      append('', 'invalid:header')
+      vary(res, 'invalid:header')
     }).toThrow(/field.*contains.*invalid/)
   })
 
   it('should not allow separator " "', () => {
+    const res = getMockResponse()
     expect(() => {
-      append('', 'invalid header')
+      vary(res, 'invalid header')
     }).toThrow(/field.*contains.*invalid/)
   })
 
   it.each(['\n', '\u0080'])("should not allow non-token character '%s'", (character: string) => {
+    const res = getMockResponse()
     expect(() => {
-      append('', `invalid${character}header`)
+      vary(res, `invalid${character}header`)
     }).toThrow(/field.*contains.*invalid/)
   })
 })
 
 describe('when header empty', () => {
   it('should set value', () => {
-    expect(append('', 'Origin')).toBe('Origin')
+    const res = getMockResponse()
+    vary(res, 'Origin')
+    expect(res.getHeader('vary')).toEqual(['Origin'])
   })
 
   it('should set value with array', () => {
-    expect(append('', ['Origin', 'User-Agent'])).toBe('Origin, User-Agent')
+    const res = getMockResponse()
+    vary(res, ['Origin', 'User-Agent'])
+    expect(res.getHeader('vary')).toEqual(['Origin', 'User-Agent'])
   })
 
   it('should preserve case', () => {
-    expect(append('', ['ORIGIN', 'user-agent', 'AccepT'])).toBe('ORIGIN, user-agent, AccepT')
+    const res = getMockResponse()
+    vary(res, ['ORIGIN', 'user-agent', 'AccepT'])
+    expect(res.getHeader('vary')).toEqual(['ORIGIN', 'user-agent', 'AccepT'])
   })
 })
 
 describe('when header has values', () => {
   it('should set value', () => {
-    expect(append('Accept', 'Origin')).toBe('Accept, Origin')
+    const res = getMockResponse('Accept')
+    vary(res, 'Origin')
+    expect(res.getHeader('vary')).toEqual(['Accept', 'Origin'])
   })
 
   it('should set value with array', () => {
-    expect(append('Accept', ['Origin', 'User-Agent'])).toBe('Accept, Origin, User-Agent')
+    const res = getMockResponse('Accept')
+    vary(res, ['Origin', 'User-Agent'])
+    expect(res.getHeader('vary')).toEqual(['Accept', 'Origin', 'User-Agent'])
   })
 
   it('should not duplicate existing value', () => {
-    expect(append('Accept', 'Accept')).toBe('Accept')
+    const res = getMockResponse('Accept')
+    vary(res, 'Accept')
+    expect(res.getHeader('vary')).toEqual(['Accept'])
   })
 
   it('should compare case-insensitive', () => {
-    expect(append('Accept', 'accEPT')).toBe('Accept')
+    const res = getMockResponse('Accept')
+    vary(res, 'accEPT')
+    expect(res.getHeader('vary')).toEqual(['Accept'])
   })
 
   it('should preserve case', () => {
-    expect(append('Accept', 'AccepT')).toBe('Accept')
+    const res = getMockResponse('Accept')
+    vary(res, 'AccepT')
+    expect(res.getHeader('vary')).toEqual(['Accept'])
   })
 })
 
 describe('when *', () => {
   it('should set value', () => {
-    expect(append('', '*')).toBe('*')
+    const res = getMockResponse()
+    vary(res, '*')
+    expect(res.getHeader('vary')).toEqual(['*'])
   })
 
   it('should act as if all values already set', () => {
-    expect(append('*', 'Origin')).toBe('*')
+    const value = ['*']
+    const res = getMockResponse(value)
+    vary(res, 'Origin')
+    expect(res.getHeader('vary')).toBe(value)
   })
 
-  it('should erradicate existing values', () => {
-    expect(append('Accept, Accept-Encoding', '*')).toBe('*')
+  it('should eradicate existing values', () => {
+    const res = getMockResponse('Accept, Accept-Encoding')
+    vary(res, '*')
+    expect(res.getHeader('vary')).toEqual(['*'])
   })
 
   it('should update bad existing header', () => {
-    expect(append('Accept, Accept-Encoding, *', 'Origin')).toBe('*')
+    const res = getMockResponse('Accept, Accept-Encoding, *')
+    vary(res, 'Origin')
+    expect(res.getHeader('vary')).toEqual(['*'])
   })
 })
 
 describe('when field is string', () => {
   it('should set value', () => {
-    expect(append('', 'Accept')).toBe('Accept')
+    const res = getMockResponse()
+    vary(res, 'Accept')
+    expect(res.getHeader('vary')).toEqual(['Accept'])
   })
 
   it('should set value when vary header', () => {
-    expect(append('', 'Accept, Accept-Encoding')).toBe('Accept, Accept-Encoding')
+    const res = getMockResponse()
+    vary(res, 'Accept, Accept-Encoding')
+    expect(res.getHeader('vary')).toEqual(['Accept', 'Accept-Encoding'])
   })
 
-  it('should acept LWS', () => {
-    expect(append('', '  Accept     ,     Origin    ')).toBe('Accept, Origin')
+  it('should accept LWS', () => {
+    const res = getMockResponse()
+    vary(res, '  Accept     ,     Origin    ')
+    expect(res.getHeader('vary')).toEqual(['Accept', 'Origin'])
   })
 
   it('should handle contained *', () => {
-    expect(append('', 'Accept,*')).toBe('*')
+    const res = getMockResponse()
+    vary(res, 'Accept,*')
+    expect(res.getHeader('vary')).toEqual(['*'])
   })
 })
 
 describe('when field is array', () => {
   it('should set value', () => {
-    expect(append('', ['Accept', 'Accept-Language'])).toBe('Accept, Accept-Language')
+    const res = getMockResponse()
+    vary(res, ['Accept', 'Accept-Language'])
+    expect(res.getHeader('vary')).toEqual(['Accept', 'Accept-Language'])
   })
 
   it('should ignore double-entries', () => {
-    expect(append('', ['Accept', 'Accept'])).toBe('Accept')
+    const res = getMockResponse()
+    vary(res, ['Accept', 'Accept'])
+    expect(res.getHeader('vary')).toEqual(['Accept'])
   })
 
   it('should be case-insensitive', () => {
-    expect(append('', ['Accept', 'ACCEPT'])).toBe('Accept')
+    const res = getMockResponse()
+    vary(res, ['Accept', 'ACCEPT'])
+    expect(res.getHeader('vary')).toEqual(['Accept'])
   })
 
   it('should handle contained *', () => {
-    expect(append('', ['Origin', 'User-Agent', '*', 'Accept'])).toBe('*')
+    const res = getMockResponse()
+    vary(res, ['Origin', 'User-Agent', '*', 'Accept'])
+    expect(res.getHeader('vary')).toEqual(['*'])
   })
 
   it('should handle existing values', () => {
-    expect(append('Accept, Accept-Encoding', ['origin', 'accept', 'accept-charset'])).toBe(
-      'Accept, Accept-Encoding, origin, accept-charset'
-    )
+    const res = getMockResponse('Accept, Accept-Encoding')
+    vary(res, ['origin', 'accept', 'accept-charset'])
+    expect(res.getHeader('vary')).toEqual(['Accept, Accept-Encoding', 'origin', 'accept-charset'])
   })
 })