ext/posix: validate mode argument in posix_mkfifo()#21102
ext/posix: validate mode argument in posix_mkfifo()#21102arshidkv12 wants to merge 10 commits intophp:masterfrom
Conversation
|
kind of make sense, |
| RETURN_FALSE; | ||
| } | ||
|
|
||
| if (mode < 0 || (mode & ~0777)) { |
There was a problem hiding this comment.
... however I m unsure the mask is correct , should not it be ~07777 ?
There was a problem hiding this comment.
0777 -> rwx rwx rwx
There was a problem hiding this comment.
I double checked, with FIFO these extra bits, while not wrong, are no-op. Your agument is valid. I ll defer the decision to the maintainer(s).
|
Looking good except I tend to disagree where the test is. Ideally, it would have its own dedicated test (and testing more broadly the boundaries). |
|
Looking good now ; let's it cool down for a while though. |
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
Is the max mode really 0o7777?
|
|
||
| if (mode < 0 || (mode & ~07777)) { | ||
| zend_argument_value_error( | ||
| 2, |
There was a problem hiding this comment.
nit: I do not think you need to format this code this way, the message is not that long
|
Thanks ! |
|
@devnexen this is a BC break, please revert. We still haven't made a decision about merging those - I guess you probably forgot that we have got bunch of PR's waiting for such decision. |
|
ok done ! |
5 participants
Validate the mode argument in
posix_mkfifo()and throw aValueErrorfor invalid permission values.