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

URLs with double slash at the start of the path fail to be requested #231

Closed
dd32 opened this issue Aug 18, 2016 · 6 comments
Closed

URLs with double slash at the start of the path fail to be requested #231

dd32 opened this issue Aug 18, 2016 · 6 comments
Milestone

Comments

@dd32
Copy link
Member

dd32 commented Aug 18, 2016

Requests_IRI currently rejects a url such as http://example.org//path/ as being valid, but accepts http://example.org/path//path/.

This seems to stem from this bit of code: https://github.com/rmccue/Requests/blob/master/library/Requests/IRI.php#L691-L694

I haven't looked at it close enough to determine if it's a logic error in the if statement, or if it's an edgecase it's specifically not attempting to support. At first I thought it was trying to catch the case where the scheme separator was picked up as the path component.

This was reported as a regression in WordPress 4.6, as we previously handled this scenario before we switched to Requests.

@rmccue
Copy link
Collaborator

rmccue commented Aug 30, 2016

I'm unsure why this code exists here; I suspect it's an intricacy in the IRI spec. I didn't write this code however, this is ported from SimplePie's IRI parser, which was written by @gsnedders. I'm wondering if it's meant to be caught at a higher level.

We can probably fix this with no real issues.

@rmccue
Copy link
Collaborator

rmccue commented Aug 30, 2016

Tracked back to this commit.

@orange-themes
Copy link

Seems like not only double path, we get the same error, when we try to access http://www.geoplugin.net/json.gp?ip=8.8.8.8

The response is like http://grab.by/Sq3i and this happened direct after wp update to 4.6

@rmccue
Copy link
Collaborator

rmccue commented Aug 30, 2016

@orange-themes I think this is unrelated; might be #232 if the server you're connecting to checks referer headers.

@gsnedders
Copy link

@rmccue Pretty sure I just misread the RFC, because this should totally work. More seriously, we probably just want to follow this given it roughly matches what browsers do.

@rmccue
Copy link
Collaborator

rmccue commented Aug 30, 2016

@gsnedders Thanks for the confirmation; my guess is it was just a double-check on higher-level normalisation. Will fix now :)

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

No branches or pull requests

4 participants