Skip to content
Draft
Show file tree
Hide file tree
Changes from 4 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
42 changes: 38 additions & 4 deletions lib/interceptor/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,43 @@ const DecoratorHandler = require('../handler/decorator-handler')
const { InvalidArgumentError, InformationalError } = require('../core/errors')
const maxInt = Math.pow(2, 31) - 1

function addHostHeader (headers, host) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, you don't need to parse the headers by yourself, this is done by undici automatically, we just need to standardise the way that these are forward to undici.

If Array, push host to the headers array, if an object, attach host as a new property

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've simplified the code!

A header case I'm slightly worried about though is non-array iterables.

request({ origin, path: '/', method: 'GET', headers: new Set([['f0', 'b0'], ['f1', 'b1']]) }).

These headers are valid under UndiciHeaders and a request like this (though rare) is possible; but the proposed array/object check would break this.

Another issue I've noticed is [string,string][] headers always raise an error due to the way header processing is handled in Request.js. Say headers = [['f0', 'b0']] or [['f0', 'b0'], ['f1', 'b1']]

if (Array.isArray(headers)) {
      if (headers.length % 2 !== 0) {
        throw new InvalidArgumentError('headers array must be even')
      }
      for (let i = 0; i < headers.length; i += 2) {
        processHeader(this, headers[i], headers[i + 1])
      }
}

if (headers == null) return { host }

if (Array.isArray(headers)) {
// Distinguish flat internal array vs array-of-pairs
if (headers.length > 0 && Array.isArray(headers[0])) {
let hasHost = false
for (const [k] of headers) {
if (k != null && String(k).toLowerCase() === 'host') { hasHost = true; break }
}
return hasHost ? headers : [['host', host], ...headers]
}

for (let i = 0; i < headers.length; i += 2) {
if (headers[i] != null && String(headers[i]).toLowerCase() === 'host') return headers
}
return ['host', host, ...headers]
}

if (headers && typeof headers[Symbol.iterator] === 'function') {
const pairs = []
let hasHost = false
for (const [k, v] of headers) {
if (k != null) {
if (String(k).toLowerCase() === 'host') hasHost = true
pairs.push([k, v])
}
}
return (hasHost ? pairs : [['host', host], ...pairs]).values()
}

for (const k of Object.keys(headers)) {
if (String(k).toLowerCase() === 'host') return headers
}
return { host, ...headers }
}

class DNSInstance {
#maxTTL = 0
#maxItems = 0
Expand Down Expand Up @@ -411,10 +448,7 @@ module.exports = interceptorOpts => {
...origDispatchOpts,
servername: origin.hostname, // For SNI on TLS
origin: newOrigin.origin,
headers: {
host: origin.host,
...origDispatchOpts.headers
}
headers: addHostHeader(origDispatchOpts.headers, origin.host)
}

dispatch(
Expand Down
71 changes: 71 additions & 0 deletions test/interceptors/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -1936,3 +1936,74 @@ test('#3951 - Should handle lookup errors correctly', async t => {
origin: 'http://localhost'
}), new Error('lookup error'))
})

test('Various (parameterized) header shapes should work with DNS interceptor', async t => {
const server = createServer({ joinDuplicateHeaders: true })
server.on('request', (req, res) => {
t.equal(req.headers.foo, 'bar')
t.equal(typeof req.headers['0'], 'undefined')
t.match(req.headers.host, /^localhost:\d+$/)
res.end('ok')
})

server.listen(0)
await once(server, 'listening')

const origin = `http://localhost:${server.address().port}`

const { cache: cacheInterceptor, dns: dnsInterceptor } = interceptors

function * genPairs () { yield ['foo', 'bar'] }

const cases = [
{
name: 'record',
headers: { foo: 'bar' },
interceptors: [dnsInterceptor()]
},
{
name: 'flat array',
headers: ['foo', 'bar'],
interceptors: [dnsInterceptor()]
},
{
name: 'record with multi-value',
headers: { foo: ['bar'] },
interceptors: [dnsInterceptor()]
},
{
name: 'iterable generator of pairs',
headers: genPairs(),
interceptors: [cacheInterceptor(), dnsInterceptor({
lookup: (_origin, _opts, cb) => cb(null, [{ address: '127.0.0.1', family: 4 }])
})]
},
{
name: 'set of pairs',
headers: new Set([['foo', 'bar']]),
interceptors: [cacheInterceptor(), dnsInterceptor({
lookup: (_origin, _opts, cb) => cb(null, [{ address: '127.0.0.1', family: 4 }])
})]
},
{
name: 'map of pairs (single)',
headers: new Map([['foo', 'bar']]),
interceptors: [cacheInterceptor(), dnsInterceptor({
lookup: (_origin, _opts, cb) => cb(null, [{ address: '127.0.0.1', family: 4 }])
})]
}
]

t = tspl(t, { plan: cases.length * 4 })

for (const c of cases) {
const agent = new Agent().compose(c.interceptors)
const r = await agent.request({ origin, path: '/', method: 'GET', headers: c.headers })
t.equal(r.statusCode, 200, c.name)
await r.body.text()
await agent.close()
}

server.close()
await once(server, 'close')
})
Loading