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

Ensure file URIs without a host build with slashes #10

Merged
merged 1 commit into from
Oct 27, 2016
Merged

Ensure file URIs without a host build with slashes #10

merged 1 commit into from
Oct 27, 2016

Conversation

matt-allan
Copy link
Contributor

If you parse a file uri without a host like file:///etc/hosts, it will rebuild as file:/etc/hosts. This PR fixes Uri\build to add the missing // for file URIs that don't have a host.

The docs for parse_url say:

This function is intended specifically for the purpose of parsing URLs and not URIs. However, to comply with PHP's backwards compatibility requirements it makes an exception for the file:// scheme where triple slashes (file:///...) are allowed. For any other scheme this is invalid.

I figure if parse_url supports file:// URIs it makes sense for Uri\build to support them too.

I tried to find a source for the proper behavior. RFC 3986 just says:

For example, the "file" URI scheme is defined so that no authority, an empty host, and "localhost" all mean the end-user's machine

@matt-allan matt-allan changed the title Ensure file URIs without hosts build with slashes Ensure file URIs without a host build with slashes Oct 27, 2016
@evert
Copy link
Member

evert commented Oct 27, 2016

Interesting! I wonder if file: is exceptional, or if there's others like it. I want to do a bit of research about that before applying this patch, but everything looks good!

@evert evert merged commit e203604 into sabre-io:master Oct 27, 2016
@matt-allan
Copy link
Contributor Author

Thanks for the quick response 😄

I'm currently depending on the 1.x release. Should I open a separate PR targeting the 1.0 branch?

@evert
Copy link
Member

evert commented Oct 27, 2016

I can do a backport. Takes only a few minutes.

@matt-allan
Copy link
Contributor Author

Awesome 👌 thanks Evert!

evert added a commit that referenced this pull request Oct 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants