-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12515633/0001-make-sure-the-client-password-isn-t-corrupted.patch
against trunk revision 1244776.

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

-1 patch. The patch command could not apply the patch.

Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/959//console

This message is automatically generated.

As before, this patch is cut against 3.3.4 and not 3.4.x.

Hi Mike, is this issue in 3.3/3.4/3.5? Then typically we'd need patch(es) that apply to each cleanly. Would you mind creating such a patch? (3.4/3.5 may only require a single patch, not sure).

Patches cut against current branch-3.3, branch-3.4, and trunk

Examining the code, I have no reason to believe the problem is fixed in 3.4 or trunk. I haven't tested there, however; we're seeing this in production with 3.3.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12516244/3.5-0001-make-sure-the-client-password-isn-t-corrupted.patch
against trunk revision 1294000.

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

-1 patch. The patch command could not apply the patch.

Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/964//console

This message is automatically generated.

shrug I can't reproduce the patch apply failure, I just cut those patches from the heads of the three branches and I can apply them without a hunk failure.

Sorry for the trouble Mike. I believe the issue is that you are using git without the "--no-prefix" option. I typically do

git diff --no-prefix HEAD^..HEAD

from the same directly you're currently using (toplevel, ie the one containing "src", as in src/contrib/..."). Notice the original patch has a/src/con.. or b/src/con... which svn/patch don't like. Could you give that a try?

Thanks!

Cut a -p0 patch instead of a -p1 patch (patch is the same for all three branches).

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12516255/0001-make-sure-the-client-password-isn-t-corrupted.patch
against trunk revision 1294000.

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

+1 core tests. The patch passed core unit tests.

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/966//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/966//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/966//console

This message is automatically generated.

Patch is applying cleanly now, however would you mind adding a test that verifies this case? That will ensure we don't regress in future versions. Thanks!

In writing the test, I discovered the real problem: zookeeper-c treats the session password as a \0-terminated string, but the java generates the session password as a 16-byte binary blob, where \0 is data. Some ideas for fixing that are 1) break the c api and add a passwd_len to the clientid_t or 2) change ZooKeeperServer.generatePasswd to not make \0. Which is better is up to you, I think.

Currently everything that uses the C api and does not pass in a session password will randomly break if they expect session passwords to work (since the password will be truncated to the null every time it crosses the C<=>Python interface). I would expect the perl api has the same problem. With this patch, the password will not be truncated (though the tradeoff is you must pass a 16-byte string from python).

It is kind of bad that this patch has been sitting here for this long. We should have another look at this issue and reconsider the solution proposed here.