From ac38ad19b1caee656c13854aa7c78eb3ebff240e Mon Sep 17 00:00:00 2001
From: Yagiz Nizipli <yagiz@nizipli.com>
Date: Wed, 16 Nov 2022 16:04:44 -0500
Subject: [PATCH] perf(fetch): simplify url serializer

---
 lib/fetch/dataURL.js    | 69 ++---------------------------------------
 test/fetch/data-uri.js  | 12 +++++++
 test/node-fetch/main.js | 22 -------------
 3 files changed, 14 insertions(+), 89 deletions(-)

diff --git a/lib/fetch/dataURL.js b/lib/fetch/dataURL.js
index 940f94a3b58..74b6d9ec472 100644
--- a/lib/fetch/dataURL.js
+++ b/lib/fetch/dataURL.js
@@ -1,5 +1,6 @@
 const assert = require('assert')
 const { atob } = require('buffer')
+const { format } = require('url')
 const { isValidHTTPToken, isomorphicDecode } = require('./util')
 
 const encoder = new TextEncoder()
@@ -111,73 +112,7 @@ function dataURLProcessor (dataURL) {
  * @param {boolean} excludeFragment
  */
 function URLSerializer (url, excludeFragment = false) {
-  // 1. Let output be url’s scheme and U+003A (:) concatenated.
-  let output = url.protocol
-
-  // 2. If url’s host is non-null:
-  if (url.host.length > 0) {
-    // 1. Append "//" to output.
-    output += '//'
-
-    // 2. If url includes credentials, then:
-    if (url.username.length > 0 || url.password.length > 0) {
-      // 1. Append url’s username to output.
-      output += url.username
-
-      // 2. If url’s password is not the empty string, then append U+003A (:),
-      // followed by url’s password, to output.
-      if (url.password.length > 0) {
-        output += ':' + url.password
-      }
-
-      // 3. Append U+0040 (@) to output.
-      output += '@'
-    }
-
-    // 3. Append url’s host, serialized, to output.
-    output += decodeURIComponent(url.hostname)
-
-    // 4. If url’s port is non-null, append U+003A (:) followed by url’s port,
-    // serialized, to output.
-    if (url.port.length > 0) {
-      output += ':' + url.port
-    }
-  }
-
-  // 3. If url’s host is null, url does not have an opaque path,
-  // url’s path’s size is greater than 1, and url’s path[0]
-  // is the empty string, then append U+002F (/) followed by
-  // U+002E (.) to output.
-  // Note: This prevents web+demo:/.//not-a-host/ or web+demo:/path/..//not-a-host/,
-  // when parsed and then serialized, from ending up as web+demo://not-a-host/
-  // (they end up as web+demo:/.//not-a-host/).
-  // Undici implementation note: url's path[0] can never be an
-  // empty string, so we have to slightly alter what the spec says.
-  if (
-    url.host.length === 0 &&
-    url.pathname.length > 1 &&
-    url.href.slice(url.protocol.length + 1)[0] === '.'
-  ) {
-    output += '/.'
-  }
-
-  // 4. Append the result of URL path serializing url to output.
-  output += url.pathname
-
-  // 5. If url’s query is non-null, append U+003F (?),
-  // followed by url’s query, to output.
-  if (url.search.length > 0) {
-    output += url.search
-  }
-
-  // 6. If exclude fragment is false and url’s fragment is non-null,
-  // then append U+0023 (#), followed by url’s fragment, to output.
-  if (excludeFragment === false && url.hash.length > 0) {
-    output += url.hash
-  }
-
-  // 7. Return output.
-  return output
+  return format(url, { fragment: !excludeFragment })
 }
 
 // https://infra.spec.whatwg.org/#collect-a-sequence-of-code-points
diff --git a/test/fetch/data-uri.js b/test/fetch/data-uri.js
index 291b7576f8b..94aa7eafcc6 100644
--- a/test/fetch/data-uri.js
+++ b/test/fetch/data-uri.js
@@ -177,3 +177,15 @@ test('too long base64 url', async (t) => {
     t.fail(`failed to fetch ${dataURL}`)
   }
 })
+
+test('https://domain.com/#', (t) => {
+  const domain = 'https://domain.com/#'
+  const serialized = URLSerializer(new URL(domain))
+  t.equal(serialized, domain)
+})
+
+test('https://domain.com/?', (t) => {
+  const domain = 'https://domain.com/?'
+  const serialized = URLSerializer(new URL(domain))
+  t.equal(serialized, domain)
+})
diff --git a/test/node-fetch/main.js b/test/node-fetch/main.js
index f4f967cf67e..08eb26670e6 100644
--- a/test/node-fetch/main.js
+++ b/test/node-fetch/main.js
@@ -1532,17 +1532,6 @@ describe('node-fetch', () => {
     })
   })
 
-  it('should NOT keep `?` sign in URL when no params are given', () => {
-    const url = `${base}question?`
-    const urlObject = new URL(url)
-    const request = new Request(urlObject)
-    return fetch(request).then(res => {
-      expect(res.url).to.equal(url.slice(0, -1))
-      expect(res.ok).to.be.true
-      expect(res.status).to.equal(200)
-    })
-  })
-
   it('if params are given, do not modify anything', () => {
     const url = `${base}question?a=1`
     const urlObject = new URL(url)
@@ -1554,17 +1543,6 @@ describe('node-fetch', () => {
     })
   })
 
-  it('should NOT preserve the hash (#) symbol', () => {
-    const url = `${base}question?#`
-    const urlObject = new URL(url)
-    const request = new Request(urlObject)
-    return fetch(request).then(res => {
-      expect(res.url).to.equal(url.slice(0, -2))
-      expect(res.ok).to.be.true
-      expect(res.status).to.equal(200)
-    })
-  })
-
   it('should support reading blob as text', () => {
     return new Response('hello')
       .blob()