fix parsing of port

[jsumners]

See the results of https://github.com/fastify/fastify-url-data/actions/runs/9907890408/job/27372511339?pr=147

I don't know why the AJV stuff is failing.

[zekth]

I'm confused also because our compatibility tests have this case:

fast-uri/test/compatibility.test.js

Line 33 in 0655453

'http://foo.bar',

[zekth]

This assumes that we only parse http/https uri schemes. We support more than this.

Suggested change

	parsed.port = parsed.scheme === 'http' ? 80 : 443
	switch (parsed.scheme) {
	case 'http':
	parsed.port = 80
	case 'https':
	parsed.port = 443
	}

[jsumners]

This is not Go. I'd have to add in break statements. In my opinion, this is the exact scenario for a ternary.

[zekth]

It was an example, sorry i wasn't explicit. Ternary assumes that we resolves only http or https; which is not true, we support any scheme; like ftp sftp etc.

At the same time i don't feel this is a good thing because we're assuming the port based on the scheme; but the uri doesn't provide any.

[jsumners]

If this library is going to assert that it fully parses URIs, it must parse the port portion:

https://datatracker.ietf.org/doc/html/rfc3986#section-3.2

authority = [ userinfo "@" ] host [ ":" port ]

URI producers and normalizers should omit the ":" delimiter that
separates host from port if the port component is empty. Some
schemes do not allow the userinfo and/or port subcomponents.

In other words, http://example.com/ translates to:

• http
• example.com
• 80

[zekth]

That's not true. In http://example.com there's no port from the uri point of view. Automatically translating to 80 for http is an extension of the interpretation of the rfc, not what the rfc dictactes.

[jsumners]

The AJV issue comes from:

1. https://github.com/ajv-validator/ajv/blob/9050ba1359fb87cd7c143f3c79513ea7624ea443/lib/compile/resolve.ts#L77

2. fast-uri/index.js

   Line 142 in fd8aa7d

   const authority = recomposeAuthority(components, options)

3. fast-uri/lib/utils.js

   Line 227 in 8dea240

   if (typeof components.port === 'number' || typeof components.port === 'string') {

[jsumners]

I don't know how to solve it because I don't know what AJV is trying to do. This patch does not do the trick:

diff --git a/index.js b/index.js
index f075141..1485fc5 100644
--- a/index.js
+++ b/index.js
@@ -119,24 +119,26 @@ function serialize (cmpts, opts) {
   // find scheme handler
   const schemeHandler = SCHEMES[(options.scheme || components.scheme || '').toLowerCase()]
 
-  // perform scheme specific serialization
-  if (schemeHandler && schemeHandler.serialize) schemeHandler.serialize(components, options)
+  if (components.host !== undefined) {
+    // perform scheme specific serialization
+    if (schemeHandler && schemeHandler.serialize) schemeHandler.serialize(components, options)
 
-  if (components.path !== undefined) {
-    if (!options.skipEscape) {
-      components.path = escape(components.path)
+    if (components.path !== undefined) {
+      if (!options.skipEscape) {
+        components.path = escape(components.path)
 
-      if (components.scheme !== undefined) {
-        components.path = components.path.split('%3A').join(':')
+        if (components.scheme !== undefined) {
+          components.path = components.path.split('%3A').join(':')
+        }
+      } else {
+        components.path = unescape(components.path)
       }
-    } else {
-      components.path = unescape(components.path)
     }
-  }
 
-  if (options.reference !== 'suffix' && components.scheme) {
-    uriTokens.push(components.scheme)
-    uriTokens.push(':')
+    if (options.reference !== 'suffix' && components.scheme) {
+      uriTokens.push(components.scheme)
+      uriTokens.push(':')
+    }
   }
 
   const authority = recomposeAuthority(components, options)
diff --git a/lib/utils.js b/lib/utils.js
index 173b9fd..d20ed94 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -222,13 +222,14 @@ function recomposeAuthority (components, options) {
       }
     }
     uriTokens.push(host)
-  }
 
-  if (typeof components.port === 'number' || typeof components.port === 'string') {
-    uriTokens.push(':')
-    uriTokens.push(String(components.port))
+    if (typeof components.port === 'number' || typeof components.port === 'string') {
+      uriTokens.push(':')
+      uriTokens.push(String(components.port))
+    }
   }
 
+
   return uriTokens.length ? uriTokens.join('') : undefined
 };
 
diff --git a/package.json b/package.json
index f836ea3..eff7647 100644
--- a/package.json
+++ b/package.json
@@ -27,7 +27,7 @@
   },
   "devDependencies": {
     "@fastify/pre-commit": "^2.1.0",
-    "ajv": "^8.16.0",
+    "ajv": "^8.17.1",
     "benchmark": "^2.1.4",
     "coveralls": "^3.1.1",
     "snazzy": "^9.0.0",

@zekth @jasoniangreen if you have any ideas, please provide them. This issue is blocking my work on fastify/fastify#5116

[gurgunday]

I’ll take a look if I can today

[zekth]

@jsumners have you tried with urijs ?

looks like https://github.com/fastify/fastify-url-data/blob/a1cfd7b9dcebc0e6217a1e7a6df62495ae9f6f4e/test/tests.test.js#L139
this is somewhat odd behavior. We technically should not rely on the uri parser the get a port that is diverted from the scheme. I'd say url-data needs to adapt this rather that fast-uri. See my related comment: #96 (comment)

[jsumners]

@jsumners have you tried with urijs ?

No. @fastify/url-data uses fast-uri. Until some recent change, it worked as expected.

[gurgunday]

Interesting

fastify/fastify-url-data#146 was fine too, so it's not a fast-uri major thing, I wonder if Fastify v5 changed what hostname returns