Strip extra null characters in socketAddress #2145
Conversation
|
|
||
| string name = Encoding.Default.GetString (bytes); | ||
| string name = Encoding.Default.GetString (bytes, 2, socketAddress.Size - 2 - 1); |
There was a problem hiding this comment.
bytes doesn't exist here as you deleted it above.
|
Can you please add the unit test to https://github.com/mono/mono/tree/master/mcs/class/Mono.Posix/Test/Mono.Unix? (we don't use the |
|
Thanks for the review. Yes, I will be glad to fix this up. |
dlech
force-pushed
the
patch-2
branch
3 times, most recently
from
fc38c5e
to
6f7485c
Compare
|
Well, I added some unit tests, but it turns out that my fix doesn't work. I am getting two extra random characters when running the unit test instead of two extra null characters. I'll have to do some more digging to figure it out - unless someone else has any insight. |
dlech
force-pushed
the
patch-2
branch
4 times, most recently
from
0a386c4
to
51bfbd1
Compare
|
OK. Should be good now. I've added a unit test for the original problem that I was seeing (UnixListenerTest) and a unit test for the method that was actually changed (UnixEndPointTest). Both of these tests fail on mono master. Then I added a separate commit with a new fix that is more robust than the previous one. |
|
build |
|
Looks good to me! I've triggered a build on the CI (expect a few test cases failing right now, they're unrelated). Any other comments from people more familiar with the code? |
|
The build failed because I was using Path.GetFullPath() in one of the unit tests and the build server has a very long path name where the tests are running. Unix sockets are limited to 108 characters in the path (see |
|
build |
|
It looks good to me. @jonpryor can you please take a look as well? |
|
@dlech would you mind rebasing this to get rid of the merge conflicts? |
In some cases, the null terminator in the SocketAddress may not be the last character. This changes the algorithm to ignore the null terminator and anything after it instead of assuming that the last character is the null terminator. Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=35004
|
rebased |
|
build |
|
LGTM. |
Strip extra null characters in socketAddress
Strip extra null characters in socketAddress Commit migrated from mono/mono@7503fd6
Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=35004.
Also remove unnecessary copying of array.