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

[BUGFIX] Add '?' for query string during URI generation #425

Merged
merged 1 commit into from
Jun 5, 2017

Conversation

erayd
Copy link
Contributor

@erayd erayd commented May 18, 2017

What

Prefixes non-empty query strings with ? when building a URI.

Why

Bugfix for #424 - URI splitting / combining should be a reversable operation.

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.

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.
Copy link
Collaborator

@bighappyface bighappyface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@bighappyface bighappyface merged commit 5dcbe1d into jsonrainbow:6.0.0-dev Jun 5, 2017
erayd added a commit to erayd/json-schema that referenced this pull request Jun 5, 2017
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 pushed a commit that referenced this pull request Jun 5, 2017
* 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.
@erayd erayd deleted the bugfix-424-query branch June 8, 2017 21:54
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

Successfully merging this pull request may close these issues.

2 participants