Skip to content
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.

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

Translated Guava Hashing implementation returns different values when called successively #797

Closed
safetydank opened this issue Sep 23, 2016 · 4 comments
Assignees

Comments

@safetydank
Copy link

I've found this method returns different values when called successively, using j2objc v1.1. The same method returns consistent values in Java (tested on Android).

public static String testHash(String input) {
byte[] bytes = input.getBytes("UTF-8");
return Hashing.sha1().hashBytes(bytes).toString();
}

assert(testHash("foo").equals(testHash("foo")) == true);

@tomball
Copy link
Collaborator

tomball commented Sep 23, 2016

I'm away from my desk until Monday, but reviewing the code I think I see the problem. In IosSHAMessageDigest.java, line 66, the initial digest buffer is declared on the stack but not cleared, so its initial values are likely to differ between invocations (depending on what was previously on the stack).

If you have a local j2objc build and have time to experiment, try adding a memset() call after the digest declaration and before the CC_SHA1() call:

protected native byte[] engineDigest() /*-[
  IOSByteArray *bytes = [buffer_ toByteArray];
  unsigned char digest[CC_SHA1_DIGEST_LENGTH];
  memset(digest, 0, CC_SHA1_DIGEST_LENGTH);  // Add this line.
  CC_SHA1(bytes->buffer_, bytes->size_, digest);
  return [IOSByteArray arrayWithBytes:(jbyte *)digest count:CC_SHA1_DIGEST_LENGTH];
]-*/;

@tomball tomball self-assigned this Sep 23, 2016
@safetydank
Copy link
Author

Thanks for looking into this. I won't have time today but will update here if I get a chance to try the fix.

@tomball
Copy link
Collaborator

tomball commented Sep 27, 2016

The memset() call wasn't necessary. The issue turns out to be an incomplete clone() method in IosSHAMessageDigest, which com.google.common.hash uses but not our unit tests.

tomball added a commit that referenced this issue Sep 28, 2016
	Change on 2016/09/28 by tball <tball@google.com>

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=134560133
@tomball
Copy link
Collaborator

tomball commented Sep 28, 2016

Fixed in current source.

@tomball tomball closed this as completed Sep 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants