Skip to content

ticket-78385 an empty string in parse_url() return when question mark… #5078

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

Conversation

fu22ybear
Copy link
Contributor

… is the last

bug ticket 78385

@carusogabriel
Copy link
Contributor

I know that this was report as a bug, but is there any change that this would break routers libraries or even frameworks?

@cmb69
Copy link
Member

cmb69 commented Jan 11, 2020

@carusogabriel looks so

@fu22ybear
Copy link
Contributor Author

I missed this test files, they have to be updated too

@cmb69
Copy link
Member

cmb69 commented Jan 11, 2020

@Islam93, ah thanks! While I agree that it's more correct to report an empty query as empty string instead of null, this might break existing code.

@fu22ybear
Copy link
Contributor Author

@cmb69, even it will not be merged, this issue was useful for me like first try to contribute to php :)

@nikic
Copy link
Member

nikic commented Jan 11, 2020

If nothing else, this change should be fine to merge into master.

@fu22ybear
Copy link
Contributor Author

I'd prefer to do both in the same PR, to make sure that the behavior between these two cases stays consistent.

@nikic, ok, I will update current PR

@fu22ybear fu22ybear changed the base branch from PHP-7.3 to master January 11, 2020 20:22
@fu22ybear fu22ybear force-pushed the parse_url_does_not_include_query_when_question_mark_is_the_last branch from 52c1be5 to 90a83e2 Compare January 11, 2020 20:41
@fu22ybear fu22ybear requested a review from nikic January 12, 2020 10:53
@php-pulls php-pulls closed this in f553e67 Jan 13, 2020
@BenMorel
Copy link
Contributor

Thank you! 👍

@carusogabriel carusogabriel added this to the PHP 8.0 milestone May 29, 2020
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.

5 participants