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

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

Closed
seshness opened this issue Oct 8, 2019 · 4 comments
Closed

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

seshness opened this issue Oct 8, 2019 · 4 comments

Comments

@seshness
Copy link

seshness commented Oct 8, 2019

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
Copy link
Member

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
Copy link
Member

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

@DaRaFF
Copy link

DaRaFF commented Oct 22, 2019

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
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants