refactor: replace follow-redirects with native HTTP

Author: thaaddeus

package-lock.json

@@ -69,11 +69,9 @@
   "license": "MPL-2.0",
   "dependencies": {
     "debug": "^4.4.3",
-    "follow-redirects": "^1.16.0",
     "ws": "^8.21.1"
   },
   "devDependencies": {
-    "@types/follow-redirects": "^1.14.4",
     "@types/node": "^26.1.1",
     "@types/ws": "^8.18.1",
     "prettier": "3.8.3",

package.json

@@ -1,14 +1,13 @@
 import crypto, { createHash } from 'crypto'
 import { createWriteStream, mkdirSync, rmSync } from 'node:fs'
 import { rename } from 'node:fs/promises'
-import { Agent as HttpAgent } from 'node:http'
-import { Agent as HttpsAgent, type RequestOptions } from 'node:https'
-import followRedirects from 'follow-redirects'
+import { Agent as HttpAgent, request as httpRequest, type IncomingMessage } from 'node:http'
+import { Agent as HttpsAgent, request as httpsRequest, type RequestOptions } from 'node:https'
+import { pipeline } from 'node:stream/promises'
 import path from 'path'
 import { debug } from './debug.ts'
 import { Mapper } from './mappers/index.ts'
 import { Disconnect, Exchange, Filter, FilterForExchange } from './types.ts'
-const { http: followRedirectsHttp, https: followRedirectsHttps } = followRedirects
 
 export function parseAsUTCDate(val: string) {
   // Treat date-only and minute-level strings as UTC instead of local time.
@@ -456,44 +455,75 @@ async function requestViaFetch(method: string, url: string, options: HttpRequest
   }
 }
 
-async function requestViaProxy(method: string, url: string, options: HttpRequestOptions): Promise<HttpResponse> {
-  const requestClient = new URL(url).protocol === 'http:' ? followRedirectsHttp : followRedirectsHttps
-  const preparedRequest = prepareRequest(method, options)
+type DownloadRequestOptions = {
+  headers: Record<string, string>
+  timeout: number
+}
 
-  return await new Promise<HttpResponse>((resolve, reject) => {
-    const request = requestClient
-      .request(
-        url,
-        {
-          method,
-          agent: getProxyAgent(url),
-          headers: preparedRequest.headers,
-          timeout: options.timeout
-        },
-        (response) => {
-          response.setEncoding('utf8')
-          let body = ''
-          response.on('error', reject)
-          response.on('data', (chunk) => (body += chunk))
-          response.on('end', () => {
-            resolve(createHttpResponse(response.statusCode ?? 0, parseNodeResponseHeaders(response.headers), body))
-          })
-        }
-      )
-      .on('error', reject)
-      .on('timeout', () => {
-        reject(new Error('Request timed out'))
-        request.destroy()
-      })
-
-    if (preparedRequest.body !== undefined) {
-      request.write(preparedRequest.body)
-    }
+function sendHttpRequest(url: string | URL, options: RequestOptions, body?: string): Promise<IncomingMessage> {
+  const requestUrl = new URL(url)
+  const requestClient = requestUrl.protocol === 'http:' ? httpRequest : httpsRequest
+  const agent = getProxyAgent(requestUrl) ?? (requestUrl.protocol === 'https:' ? httpsAgent : undefined)
+
+  return new Promise((resolve, reject) => {
+    const request = requestClient(requestUrl, { ...options, agent }, resolve)
 
+    request.once('error', reject)
+    request.once('timeout', () => request.destroy(new Error('Request timed out')))
+    if (body !== undefined) {
+      request.write(body)
+    }
     request.end()
   })
 }
 
+async function openDownloadResponse(url: string, options: DownloadRequestOptions) {
+  const response = await sendHttpRequest(url, options)
+  const statusCode = response.statusCode ?? 0
+  const location = response.headers.location
+  if (location === undefined || statusCode < 300 || statusCode >= 400) {
+    return response
+  }
+
+  response.destroy()
+  const sourceUrl = new URL(url)
+  const redirectUrl = new URL(location, sourceUrl)
+  const headers: Record<string, string> = { ...options.headers }
+  if (sourceUrl.origin !== redirectUrl.origin) {
+    delete headers.Authorization
+    delete headers.authorization
+  }
+
+  return sendHttpRequest(redirectUrl, {
+    ...options,
+    headers
+  })
+}
+
+async function readResponseText(response: IncomingMessage) {
+  response.setEncoding('utf8')
+  let body = ''
+  for await (const chunk of response) {
+    body += chunk
+  }
+  return body
+}
+
+async function requestViaProxy(method: string, url: string, options: HttpRequestOptions): Promise<HttpResponse> {
+  const preparedRequest = prepareRequest(method, options)
+  const response = await sendHttpRequest(
+    url,
+    {
+      method,
+      headers: preparedRequest.headers,
+      timeout: options.timeout
+    },
+    preparedRequest.body
+  )
+
+  return createHttpResponse(response.statusCode ?? 0, parseNodeResponseHeaders(response.headers), await readResponseText(response))
+}
+
 async function request(method: string, url: string, options: HttpRequestOptions = {}) {
   const retrySettings = getRetrySettings(method, options.retry)
 
@@ -622,7 +652,12 @@ export function cleanTempFiles() {
   tmpFileCleanups.forEach((cleanup) => cleanup())
 }
 
-async function _downloadFile(requestOptions: RequestOptions, url: string, downloadPath: string, appendContentEncodingExtension: boolean) {
+async function _downloadFile(
+  requestOptions: DownloadRequestOptions,
+  url: string,
+  downloadPath: string,
+  appendContentEncodingExtension: boolean
+) {
   // first ensure that directory where we want to download file exists
   mkdirSync(path.dirname(downloadPath), { recursive: true })
 
@@ -642,59 +677,28 @@ async function _downloadFile(requestOptions: RequestOptions, url: string, downlo
 
   try {
     // based on https://github.com/nodejs/node/issues/28172 - only reliable way to consume response stream and avoiding all the 'gotchas'
-    let responseHeaders: Record<string, string> = {}
-    await new Promise<void>((resolve, reject) => {
-      const protocol = new URL(url).protocol
-      const requestClient = protocol === 'http:' ? followRedirectsHttp : followRedirectsHttps
-      const agent = getProxyAgent(url) ?? (protocol === 'https:' ? httpsAgent : undefined)
-      const req = requestClient
-        .get(url, { ...requestOptions, agent }, (res) => {
-          const { statusCode } = res
-          if (statusCode !== 200) {
-            // read the error response text and throw it as an HttpError
-            res.setEncoding('utf8')
-            let body = ''
-            res.on('error', reject)
-            res.on('data', (chunk) => (body += chunk))
-            res.on('end', () => {
-              reject(new HttpError(statusCode!, body, url))
-            })
-          } else {
-            responseHeaders = parseNodeResponseHeaders(res.headers)
-            if (appendContentEncodingExtension) {
-              const contentEncoding = asSingleHeaderValue(res.headers['content-encoding'])
-              if (contentEncoding === 'zstd') {
-                finalDownloadPath = `${downloadPath}.zst`
-              } else if (contentEncoding === undefined || contentEncoding === 'gzip') {
-                finalDownloadPath = `${downloadPath}.gz`
-              } else {
-                reject(new Error(`Unsupported data feed content encoding: ${contentEncoding}`))
-                return
-              }
-            }
+    const response = await openDownloadResponse(url, requestOptions)
+    const { statusCode } = response
+    if (statusCode !== 200) {
+      throw new HttpError(statusCode ?? 0, await readResponseText(response), url)
+    }
 
-            // consume the response stream by writing it to the file
-            res
-              .on('error', reject)
-              .on('aborted', () => reject(new Error('Request aborted')))
-              .pipe(fileWriteStream)
-              .on('error', reject)
-              .on('finish', () => {
-                if (res.complete) {
-                  resolve()
-                } else {
-                  reject(new Error('The connection was terminated while the message was still being sent'))
-                }
-              })
-          }
-        })
-        .on('error', reject)
-        .on('timeout', () => {
-          debug('download file request timeout, %s', url)
-          reject(new Error('Request timed out'))
-          req.destroy()
-        })
-    })
+    const responseHeaders = parseNodeResponseHeaders(response.headers)
+    if (appendContentEncodingExtension) {
+      const contentEncoding = asSingleHeaderValue(response.headers['content-encoding'])
+      if (contentEncoding === 'zstd') {
+        finalDownloadPath = `${downloadPath}.zst`
+      } else if (contentEncoding === undefined || contentEncoding === 'gzip') {
+        finalDownloadPath = `${downloadPath}.gz`
+      } else {
+        throw new Error(`Unsupported data feed content encoding: ${contentEncoding}`)
+      }
+    }
+
+    await pipeline(response, fileWriteStream)
+    if (!response.complete) {
+      throw new Error('The connection was terminated while the message was still being sent')
+    }
 
     // finally when saving from the network to file has succeded, rename tmp file to normal name
     // then we're sure that responses is 100% saved and also even if different process was doing the same we're good

src/handy.ts

@@ -1,9 +1,17 @@
 import { test } from 'node:test'
+import { execFile } from 'node:child_process'
+import { mkdtempSync, readFileSync, rmSync } from 'node:fs'
 import { createServer, type IncomingMessage, type ServerResponse } from 'node:http'
 import type { AddressInfo } from 'node:net'
-import { getJSON, postJSON } from '../dist/handy.js'
+import os from 'node:os'
+import path from 'node:path'
+import { pathToFileURL } from 'node:url'
+import { promisify } from 'node:util'
+import { download, getJSON, postJSON } from '../dist/handy.js'
 import { assert } from './assertions.ts'
 
+const execFileAsync = promisify(execFile)
+
 test('retries a temporary GET failure and returns the HTTP response', async () => {
   let requestsCount = 0
   const server = await startServer((_request, response) => {
@@ -53,6 +61,54 @@ test('sends a JSON POST body and retries when requested', async () => {
   }
 })
 
+test('follows one download redirect without forwarding authorization to another origin', { timeout: 5000 }, async () => {
+  let redirectedAuthorization: string | undefined
+  const target = await startServer((request, response) => {
+    redirectedAuthorization = request.headers.authorization
+    response.writeHead(200).end('redirected data')
+  })
+  const source = await startServer((request, response) => {
+    assert.strictEqual(request.headers.authorization, 'Bearer secret')
+    response.writeHead(302, { Location: `${target.url}/data` }).end()
+  })
+  const tempDir = mkdtempSync(path.join(os.tmpdir(), 'tardis-node-http-'))
+  const downloadPath = path.join(tempDir, 'data.bin')
+
+  try {
+    await download({ url: `${source.url}/redirect`, downloadPath, userAgent: 'tardis-node-test', apiKey: 'secret' })
+
+    assert.strictEqual(redirectedAuthorization, undefined)
+    assert.strictEqual(readFileSync(downloadPath, 'utf8'), 'redirected data')
+  } finally {
+    rmSync(tempDir, { force: true, recursive: true })
+    await source.close()
+    await target.close()
+  }
+})
+
+test('routes HTTP requests through the configured proxy', { timeout: 5000 }, async () => {
+  let requestedUrl: string | undefined
+  const proxy = await startServer((request, response) => {
+    requestedUrl = request.url
+    response.writeHead(200, { 'Content-Type': 'application/json' }).end('{"proxied":true}')
+  })
+  const handyModuleUrl = pathToFileURL(path.resolve('dist/handy.js')).href
+  const script = `import { getJSON } from ${JSON.stringify(handyModuleUrl)}; process.stdout.write(JSON.stringify(await getJSON('http://exchange.test/data')))`
+
+  try {
+    const { stdout } = await execFileAsync(process.execPath, ['--input-type=module', '--eval', script], {
+      env: { ...process.env, HTTP_PROXY: proxy.url, HTTPS_PROXY: proxy.url, NO_PROXY: '' }
+    })
+
+    const result = JSON.parse(stdout)
+    assert.strictEqual(requestedUrl, 'http://exchange.test/data')
+    assert.strictEqual(result.statusCode, 200)
+    assert.deepStrictEqual(result.data, { proxied: true })
+  } finally {
+    await proxy.close()
+  }
+})
+
 async function startServer(handler: (request: IncomingMessage, response: ServerResponse) => void | Promise<void>) {
   const server = createServer((request, response) => {
     void Promise.resolve(handler(request, response)).catch((error) => response.destroy(error))