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

Allow empty authority for unknown schemes #4349

Conversation

@jfinkhaeuser
Copy link
Contributor

commented Sep 13, 2019

I discussed this briefly with @bagder

According to RFC3986 Section 3.2.2 on page 21

If the URI scheme defines a default for host, then that default
applies when the host subcomponent is undefined or when the
registered name is empty (zero length). For example, the "file" URI
scheme is defined so that no authority, an empty host, and
"localhost" all mean the end-user's machine, whereas the "http"
scheme considers a missing authority or empty host invalid.

cURL implements this behaviour for the file scheme, but doesn't - and cannot - implement this behaviour for schemes it does not know. For such schemes, passing a new flag that permits empty authority sections is desirable.

This PR adds such a CURLU_NO_AUTHORITY flag, as well as test cases.

for use with unknown schemes (i.e. not "file:///") to override cURL's
demand that an authority exists.
parts should fail. With it, it should work. The flag effectively
disables the hostname check if it's set, and the hostname is empty.
empty.
@bagder bagder added the URL label Sep 13, 2019
@jfinkhaeuser

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2019

I've got to run - if there are tests failing, or there should be changes, I'll look at them in a few days, probably. Locally everything looked fine. And obviously if you'd like changes, I'm happy to provide them.

@bagder

This comment has been minimized.

Copy link
Member

commented Sep 13, 2019

I think this is a sensible way to make the parser handle such URLs so I'm in favor of merging once things are green. Still missing: documentation of the new flag and symbols-in-versions update.

@jfinkhaeuser

This comment has been minimized.

Copy link
Contributor Author

commented Sep 15, 2019

Makes sense - will probably get to those changes tomorrow.

@bagder

This comment has been minimized.

Copy link
Member

commented Sep 15, 2019

The two builds using boringssl fail due to other reasons, not your fault!

@jfinkhaeuser

This comment has been minimized.

Copy link
Contributor Author

commented Sep 15, 2019

Excellent, then let me know if you want any other changes, otherwise I'm done.

@bagder
bagder approved these changes Sep 19, 2019
Copy link
Member

left a comment

Will merge asap!

@bagder bagder closed this in 0a4ecbd Sep 19, 2019
@bagder

This comment has been minimized.

Copy link
Member

commented Sep 19, 2019

Thanks!

@jfinkhaeuser

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2019

No worries, glad to be of help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.