Skip to content

Conversation

@stickyhipp
Copy link

Fixes a false Digest mismatch when participants are running in environments with different default locales.


CRC32 crc = new CRC32();
crc.update(path.getBytes());
crc.update(path.getBytes(StandardCharsets.UTF_8));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I find it convenient to use static imports for UTF_8, as it makes the code a bit nicer to read.

@eolivelli
Copy link
Contributor

Shall we make this value configurable? What will happen to existing clusters that are using a different encoding?

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing your fix and pointing out this issue.
I left one question.

We should also add a minimal unit test in order to prevent regressions on the future

@ctubbsii
Copy link
Member

Shall we make this value configurable? What will happen to existing clusters that are using a different encoding?

The use of the old method is already configurable... via the user's locale settings. I'm not sure how this method is used, but if the result is serialized anywhere within ZooKeeper for reuse later, ZK itself should define the encoding explicitly. That's good practice for serializing... but I don't know enough about the code to know how it is used. Just throwing in my 2 cents for consideration. 😺

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

Successfully merging this pull request may close these issues.

3 participants