-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix GH-16137: "Deduplicate" *Forwarded* http headers values. #16154
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
e2ae958 to
9b21b78
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use ZVAL_NEW_STR here, but not terribly important.
9f18bd7 to
0f034ef
Compare
0f034ef to
3a1786c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is right as far as I understand. But maybe wait for like at least one more approval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides the nits, this looks good to me.
sapi/cli/php_cli_server.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you revert the conditions, you can simplify:
This is the same as with_comma → entry == NULL (logical implication).
sapi/cli/php_cli_server.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superfluous empty line:
Those are meant to have 1 or plus values separated by a comma even if the client set them separately.
3a1786c to
ca633ac
Compare
Those are meant to have 1 or plus values separated by a comma even
if the client set them separately.