fix: improve Response.json() and Response.redirect() spec compliance and efficiency

[usualoma]

The optimizations for Response.json() and Response.redirect() introduced in #333 cause behavior discrepancies with native Response objects in cases such as those discussed at honojs/hono#2343.

While excluding the Response.json() optimization is under consideration, if it is included as is, I believe we should ensure that there are no behavioral discrepancies with the native Response object, as proposed in this pull request.

What this pull request includes

• Throw TypeError for non-JSON-serializable data in Response.json()
• Add URL validation with fast-path for common ASCII URLs in Response.redirect()
• Remove redundant type assertions in Response.json()

[yusukebe]

Hey @usualoma

I have reconsidered. First, I confirmed that the performance of Response.json will be significantly improved with the optimization. Here is one of the benchmark results:

I said these changes are unnecessary, but the improvement is significant, and the code hasn't increased much. So, let's keep the optimizations for Response.json() and Response.redirect()!

Is this PR ready for review?

[usualoma]

Hi @yusukebe

Thanks!

I hadn't really thought this through because I wasn't sure if the idea would be adopted, but writing it out with a straightforward if statement might actually be faster than using Uint8Array(128). Let me check—please hold on a moment.

[usualoma]

HI @yusukebe

After running a benchmark script (generated by AI), it seems that using regular expressions is faster in this case. Plus, the code is much shorter, so I’ve decided to go with d78a0b2.

(The rankings for “Regex (positive class)” and “Regex (positive, /i)” change with each run, so I believe this is entirely within the margin of error.)

If the review results are satisfactory, please merge.

benchmark                   avg (min … max) p75 / p99    (min … top 1%)
------------------------------------------- -------------------------------
• parseRedirectUrl
------------------------------------------- -------------------------------
Loose (charCodeAt, original) 320.12 ns/iter 325.03 ns        █▇▂
                    (292.60 ns … 392.59 ns) 365.70 ns      ▃▂███
                    (  0.10  b … 498.18  b)   1.98  b ▂▃▆███████▇▆▃▃▁▂▂▃▂▁▁

Strict + skip prefix         310.79 ns/iter 314.35 ns   ▄██▅
                    (298.80 ns … 384.41 ns) 361.75 ns ▃█████▃
                    (  0.10  b … 526.10  b)   2.35  b ███████▄▂▂▁▂▁▁▂▁▁▁▁▁▁

Regex (positive class)       124.51 ns/iter 126.19 ns   ▅▇█
                    (111.44 ns … 213.16 ns) 183.92 ns   ███
                    (  0.10  b … 363.12  b)   0.63  b ▄▃████▃▁▁▁▁▁▁▁▁▁▁▁▁▂▂

Regex (positive, /i)         152.82 ns/iter 181.20 ns                █
                    (116.28 ns … 227.22 ns) 201.33 ns  █▆            █▄
                    (  0.10  b … 363.12  b)   0.62  b ███▇▃▂▂▁▁▁▁▁▁▁▂██▆▂▁▁

redirect-full.mjs

// Benchmark: Full parseRedirectUrl — charCodeAt loop vs regex approaches
//
// Usage: node benchmarks/redirect-full.mjs

import { bench, group, run } from 'mitata'

const urls = [
  'https://example.com/',
  'https://example.com/path/to/resource?query=value&foo=bar#section',
  'https://sub.domain.example.com:8080/api/v2/users/123?token=abc123&redirect=https%3A%2F%2Fother.com',
  'http://localhost:3000/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z',
  'https://example.com/~user/path_name/file-name.html',
]

// Shared character table for charCodeAt approaches
const allowedRedirectUrlChar = new Uint8Array(128)
for (let c = 0x30; c <= 0x39; c++) allowedRedirectUrlChar[c] = 1
for (let c = 0x41; c <= 0x5a; c++) allowedRedirectUrlChar[c] = 1
for (let c = 0x61; c <= 0x7a; c++) allowedRedirectUrlChar[c] = 1
{
  const chars = "-./:?#[]@!$&'()*+,;=%~_"
  for (let i = 0; i < chars.length; i++) allowedRedirectUrlChar[chars.charCodeAt(i)] = 1
}

// --- Approach 1: Loose prefix + charCodeAt loop (original) ---
function parseLoose(url) {
  if (
    url.length > 8 &&
    url.charCodeAt(0) === 0x68 &&
    (url.charCodeAt(4) === 0x3a || url.charCodeAt(5) === 0x3a)
  ) {
    for (let i = 0, len = url.length; i < len; i++) {
      const c = url.charCodeAt(i)
      if (c > 0x7f || allowedRedirectUrlChar[c] === 0) {
        return new URL(url).href
      }
    }
    return url
  }
  return new URL(url).href
}

// --- Approach 2: Strict prefix + skip verified bytes (6e851ff) ---
function parseStrictSkipPrefix(url) {
  let start
  if (url.length > 8 && url.charCodeAt(0) === 0x68) {
    const c4 = url.charCodeAt(4)
    if (
      c4 === 0x3a &&
      url.charCodeAt(1) === 0x74 && url.charCodeAt(2) === 0x74 &&
      url.charCodeAt(3) === 0x70 &&
      url.charCodeAt(5) === 0x2f && url.charCodeAt(6) === 0x2f
    ) {
      start = 7
    } else if (
      c4 === 0x73 &&
      url.charCodeAt(1) === 0x74 && url.charCodeAt(2) === 0x74 &&
      url.charCodeAt(3) === 0x70 &&
      url.charCodeAt(5) === 0x3a && url.charCodeAt(6) === 0x2f && url.charCodeAt(7) === 0x2f
    ) {
      start = 8
    }
    if (start !== undefined) {
      let safe = true
      for (let i = start, len = url.length; i < len; i++) {
        const c = url.charCodeAt(i)
        if (c > 0x7f || allowedRedirectUrlChar[c] === 0) {
          safe = false
          break
        }
      }
      if (safe) {
        return url
      }
    }
  }
  return new URL(url).href
}

// --- Approach 3: Regex (positive char class, no /i) ---
const rePositive = /^https?:\/\/[!#-;=?-[\]_a-z~A-Z]+$/
function parseRegex(url) {
  if (rePositive.test(url)) return url
  return new URL(url).href
}

// --- Approach 4: Regex (with /i flag) ---
const reWithI = /^https?:\/\/[!#-;=?-[\]_a-z~]+$/i
function parseRegexI(url) {
  if (reWithI.test(url)) return url
  return new URL(url).href
}

// Correctness
for (const url of urls) {
  const ref = parseLoose(url)
  const results = {
    loose: ref,
    strictSkip: parseStrictSkipPrefix(url),
    regex: parseRegex(url),
    regexI: parseRegexI(url),
  }
  for (const [name, val] of Object.entries(results)) {
    if (val !== ref) {
      console.error(`MISMATCH ${name} for "${url}": got=${val} expected=${ref}`)
      process.exit(1)
    }
  }
}
console.log('Correctness check passed.\n')

// Benchmark
group('parseRedirectUrl', () => {
  bench('Loose (charCodeAt, original)', () => {
    for (const url of urls) parseLoose(url)
  })
  bench('Strict + skip prefix', () => {
    for (const url of urls) parseStrictSkipPrefix(url)
  })
  bench('Regex (positive class)', () => {
    for (const url of urls) parseRegex(url)
  })
  bench('Regex (positive, /i)', () => {
    for (const url of urls) parseRegexI(url)
  })
})

await run()

[yusukebe]

LGTM!

[yusukebe]

@usualoma Ah, it became simple. Thank you!