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.

Sign up for GitHub

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

Jump to bottom

UriResolver::generate does not properly reconstruct URLs with a query fragment. #424

Closed
TimothyBJacobs opened this issue May 17, 2017 · 3 comments
Closed

UriResolver::generate does not properly reconstruct URLs with a query fragment. #424

TimothyBJacobs opened this issue May 17, 2017 · 3 comments

Comments

@TimothyBJacobs
Copy link

The UriResolver appends the fragment to the path without a leading ? causing an incorrect URL.
Ex: http://example.org/?rest=/route becomes http://example.org/rest=/route.

https://github.com/justinrainbow/json-schema/blob/master/src/JsonSchema/Uri/UriResolver.php#L65

Happy to submit a patch.
erayd added a commit to erayd/json-schema that referenced this issue May 18, 2017
@erayd
Bugfix for jsonrainbow#424 - add '?' for query string
c017d2b 
Note that this bugfix will cause empty query strings to be dropped (e.g.
http://example.com?#blue becomes http://example.com#blue). This is
because the '?' character is deliberately not captured as part of the
query string, and the testsuite expects to be able to pass an empty
query string and *not* have the '?' added for that case.
@erayd erayd mentioned this issue May 18, 2017
Merged
@erayd
Copy link
Contributor

erayd commented May 18, 2017

Thanks for the bug report! This is fixed in #425. I will backport the fix for 5.2.2 once it's merged.

@TimothyBJacobs
Copy link
Author

Fantastic, thank you!

bighappyface pushed a commit that referenced this issue Jun 5, 2017
@erayd @bighappyface
Bugfix for #424 - add '?' for query string (#425)
5dcbe1d 
Note that this bugfix will cause empty query strings to be dropped (e.g.
http://example.com?#blue becomes http://example.com#blue). This is
because the '?' character is deliberately not captured as part of the
query string, and the testsuite expects to be able to pass an empty
query string and *not* have the '?' added for that case.
erayd added a commit to erayd/json-schema that referenced this issue Jun 5, 2017
@erayd
Bugfix for jsonrainbow#424 - add '?' for query string (jsonrainbow#425)
f120ce1 
Note that this bugfix will cause empty query strings to be dropped (e.g.
http://example.com?#blue becomes http://example.com#blue). This is
because the '?' character is deliberately not captured as part of the
query string, and the testsuite expects to be able to pass an empty
query string and *not* have the '?' added for that case.
@erayd erayd mentioned this issue Jun 5, 2017
Merged
bighappyface pushed a commit that referenced this issue Jun 5, 2017
@erayd @bighappyface
Backports for 5.2.2 (#431)
2a42039 
* Remove PHPDocumentor & change travis platform for hhvm tests (#429)

* Remove dev-time dependency on phpdocumentor due to resolution headaches

* Switch distro for hhvm testing to trusty (precise now unsupported)

* Bugfix for #424 - add '?' for query string (#425)

Note that this bugfix will cause empty query strings to be dropped (e.g.
http://example.com?#blue becomes http://example.com#blue). This is
because the '?' character is deliberately not captured as part of the
query string, and the testsuite expects to be able to pass an empty
query string and *not* have the '?' added for that case.
@bighappyface
Copy link
Collaborator

Closed with #425

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@erayd @bighappyface @TimothyBJacobs