Skip to content

Validate length on socket_write #3629

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
Closed
thiagooak wants to merge 1 commit into from
Closed

Validate length on socket_write #3629

thiagooak wants to merge 1 commit into from

Conversation

@thiagooak
Copy link
Contributor

@thiagooak thiagooak commented Oct 21, 2018

@petk petk added the Bug label Oct 21, 2018
cmb69
cmb69 reviewed Oct 21, 2018
View reviewed changes
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thanks. This looks good to me, but I'm not sure which branch to target.

KalleZ
KalleZ reviewed Oct 21, 2018
View reviewed changes
KalleZ
KalleZ reviewed Oct 21, 2018
View reviewed changes
@thiagooak
Copy link
Contributor Author

thiagooak commented Oct 22, 2018

@cmb69 what's a good way to figure out which branch to target?

@cmb69
Copy link
Member

cmb69 commented Oct 22, 2018

what's a good way to figure out which branch to target?

Bugfixes should usually target PHP-7.1. The respective ticket mentions missing checks in socket_send() and socket_sendto() as well. Have these been fixed in the meantime?

@thiagooak thiagooak changed the base branch from PHP-7.3 to PHP-7.1 October 23, 2018 05:51
@thiagooak
Copy link
Contributor Author

thiagooak commented Oct 23, 2018

@cmb69

  • moved check up. right after zend_parse_parameter
  • targeting 7.1 branch
  • covered socket_send and socket_sendto

@cmb69
Copy link
Member

cmb69 commented Oct 23, 2018

Thanks! LGTM. @sgolemon Okay to merge into PHP-7.1?

@krakjoe
Copy link
Member

krakjoe commented Nov 13, 2018

Merged ec2e7a2

Thanks.

@krakjoe krakjoe closed this Nov 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmb69 cmb69 cmb69 left review comments

+1 more reviewer

@KalleZ KalleZ KalleZ left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@thiagooak @cmb69 @krakjoe @KalleZ @petk