medialize / URI.js

Javascript URL mutation library
http://medialize.github.io/URI.js/
MIT License
6.26k stars 474 forks source link

URI with no host and no leading slash in path creates a host #387

Closed seshness closed 5 years ago

seshness commented 5 years ago

I have code that looks like

const uri = new URI('')
  .protocol('food')
  .path('test/file.csv')
  .toString()

I expected uri to be food:///test/file.csv, but it's food://test/file.csv. When re-parsing uri the host is now test and path has changed from test/file.csv to /file.csv.

Is this expected, and if so, should I ensure my path has a leading slash? Or, is this unexpected behaviour in the library?

Thanks for this library, and for your time!

rodneyrehm commented 5 years ago

I'm inclined to call this a bug. I'd say https://github.com/medialize/URI.js/blob/gh-pages/src/URI.js#L684 should be changed to

if (parts.path.charAt(0) !== '/' && t) {
rodneyrehm commented 5 years ago

thanks for reporting this! It's fixed and released as 1.19.2

DaRaFF commented 5 years ago

Hi @rodneyrehm

Do you really think this is a valid change? With version 1.19.2 I get back a triple-slash for a http URI. But when I look into the http specification only two-slashes are allowed. I guess there are quite a few people struggling with that change.

// https://host.com/something
const urijs1 = require("urijs@1.19.1")
const uri1 = urijs1('host.com/something').scheme('https').href()

// https:///host.com/something
const urijs2 = require("urijs@1.19.2")
const uri2 = urijs2('host.com/something').scheme('https').href()
rodneyrehm commented 5 years ago

Grüezi @DaRaFF ;)

unfortunately I do believe the change is valid. What you're describing is another bug that was previously hidden by the first,

URI('foo/bar') determines foo/bar to be a relative directory no matter if the first directory might look like a domain (host.com, localhost, …). This has confused people for years. In order to make URI.js parse properly you'd need to prepend :// to make it a relative-scheme url.

Maybe it's time to revisit #260 (and the unfinished attempt at providing a solution: #374) and provide a proper API for what you're trying to do… Are you looking to get your hands dirty with this?