Initialize s_un (sockaddr_un) to zero before using it#3408
Initialize s_un (sockaddr_un) to zero before using it#3408mmeyer724 wants to merge 1 commit intophp:masterfrom
Conversation
|
It looks reasonable. Would you be able to add a test for this? And pls could you open a bug? I also checked the code a little bit and it seems that it might also overflow in socket_sendto fom long paths on mac in here: I might be wrong but it seems that Anyway it's a different issue so not really related to this one... |
|
@mmeyer724 any action here ? |
|
@krakjoe Sorry I forgot about this! I'm working on bukka's feedback now |
|
@bukka Sorry for the delay, would you mind checking out the |
|
For what it's worth, I'm pretty sure the appveyor build failures are unrelated to my changes here. 😕 |
|
LGTM |
|
What is the reason that Windows is skipped here? I'm not too familiar with the socket code but I did not see any obvious places for anything not implemented on Windows in the surrounding blocks? |
|
@KalleZ I initially wrote the test without the windows exclusion, but it failed hard in the CI pipeline. As far as I can tell AF_UNIX sockets are not supported on Windows (or at least they're not supported on the version of windows appveyor is using: https://blogs.msdn.microsoft.com/commandline/2017/12/19/af_unix-comes-to-windows) |
|
Aha thanks, that clears it up a little! I was just curious given we don't have any surrounding guards for the AF_UNIX blocks in the code. Perhaps thats something we should look into, but that can wait for another issue. Alternatively you can use the userland constants ''PHP_WINDOWS_XXX'' to see if the version of Windows supports like:
(Keep in mind its just a pseudo) |
|
For what i'd care, this patch is fine. Unix socket supprot on Windows needs to be yet revisited and enabled/completed including tests. The patch seems to be fine for now for 7.2 as well. Thanks. |
|
Merged as 3c42c78. Thanks |
On MacOS 10.14 w/PHP 7.2.8, I am unable to send data between two unix datagram sockets due to a bug in PHP's
socket_recvfromimplementation. This bug caused the path/name of the sender to come back with trailing garbage bytes at the end of the string.This appears to be caused by a failure to initialize
struct sockaddr_un s_un;to 0 before passing it intorecvfrom. This pull requests simply adds a memset to set the memory to 0 first.Sample output from my program before:
Sample output with this fix in place:
Note: This script runs fine on Ubuntu 16.04 using the stock php package, so I'm assuming this bug only affects MacOS.