Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

withProtocol stips host when input is in {host}:{post} format #237

Open
sandros94 opened this issue Apr 19, 2024 · 2 comments
Open

withProtocol stips host when input is in {host}:{post} format #237

sandros94 opened this issue Apr 19, 2024 · 2 comments
Labels
bug Something isn't working discussion

Comments

@sandros94
Copy link

Environment

Node.js 20 and Bun 1.1

packages:

  • Nuxt: 3.11.2
  • ufo: 1.5.3

Reproduction

I'm sufficiently secure this is more than enough

import { withHttp, withHttps, withProtocol } from 'ufo'

export default eventHandler(async (event) => {
  const host = 'localhost:9000'
  console.log(withHttp(host))
  console.log(withHttps(host))
  console.log(withProtocol(host, 'ftp://'))
})

Describe the bug

The reproduction outputs:

http://9000
https://9000
ftp://9000

Additional context

Discovered this while building a small app that will be containerized and deployed within a docker stack.

The host (picked from the runtimeConfig) is used to define the container hostname for the backend, and since they could bind or edit the default port I wanted to let them include into the env variable that overrides the runtimeConfig, but with my surprise that it also deletes the host part.

Logs

No response

@pi0 pi0 transferred this issue from unjs/ofetch Apr 19, 2024
@pi0 pi0 changed the title bug: withProtocol, withHttp and withHttps remove host when port is present in the string host is parsed as port when input is in {host}:{post} format Apr 19, 2024
@pi0 pi0 changed the title host is parsed as port when input is in {host}:{post} format withProtocol stips host when input is in {host}:{post} format Apr 19, 2024
@pi0
Copy link
Member

pi0 commented Apr 19, 2024

Hi. This is an annoying issue and edge-case indeed but the reason is that at least by spec, localhost: can be a valid protocol and ufo cannot make a distinguish when input is in the form of host:port alone and ufo utilities do not throw an error like new URL('localhost:3000') does to require protocol.

One possible solution is that we make regex to be stricter and require :// for host:port pattern or add an exception list (*) of specific ones don't need it (such as mailto: and tel:) but it is tricky and we might consider a major version for it. thinking more.


(*) quick research for known exceptions: (as we see, this can extend)

['mailto:', 'tel:', 'urn:', 'data:', 'about:', 'javascript:', 'file:', 'blob:', 'magnet:', 'news:', 'sftp:', 'ssh:', 'feed:', 'webcal:', 'bitcoin:', 'geo:', 'sip:', 'sips:', 'sms:', 'smsto:', 'mms:', 'mmsto:', 'gtalk:']

extended list (thanks to chatgpt!)

['mailto:', 'tel:', 'urn:', 'data:', 'about:', 'javascript:', 'file:', 'blob:', 'magnet:', 'news:', 'sftp:', 'ssh:', 'feed:', 'webcal:', 'bitcoin:', 'geo:', 'sip:', 'sips:', 'sms:', 'smsto:', 'mms:', 'mmsto:', 'gtalk:', 'facetime:', 'chrome:', 'chrome-extension:', 'content:', 'dicom:', 'edis:', 'im:', 'info:', 'ldap:', 'mid:', 'gopher:', 'ni:', 'nih:', 'market:', 'moz:', 'ms-help:', 'ms-settings:', 'opera:', 'pres:', 'resource:', 'rmi:', 'rsync:', 'rtmp:', 'secondlife:', 'spotify:', 'svn:', 'tag:', 'vemmi:', 'view-source:', 'wais:', 'ws:', 'wss:', 'xfire:', 'xmlrpc.beep:', 'xmlrpc.beeps:', 'ymsgr:']

I would suggest that you change your application to either accept host and port separately or require that the single URL config always include a protocol itself in the meantime.

@pi0 pi0 added bug Something isn't working discussion labels Apr 19, 2024
@sandros94
Copy link
Author

sandros94 commented Apr 19, 2024

Sorry, I didn't notice I posted it on ofetch, I must have mixed up the tabs (as I was double checking the docs to be sure I was using it correctly).

In reality I didn't notice this bug with localhost: but with host.docker.internal: (a local domain inserted by Docker Desktop to simplify the connection between services and application in dev). In production it will change to backend-container-name (port 80 by default), or a domain if deployed separate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working discussion
Projects
None yet
Development

No branches or pull requests

2 participants