Add option of choosing key charset, enable keys with non-ascii characters

[spkrka]

No description provided.

[nresare]

I looked around a bit for a test case that actually tests non-ascii keys but failed to find it. Perhaps we should have that? (Or perhaps I'm blind)

[coveralls]

Changes Unknown when pulling ea36b41 on krka/charsets into * on master*.

[spkrka]

Yes, that should be added before we merge this. I wanted to get some more general feedback on code architecture/design first.

[nresare]

If you just want a review of the overall thinking, I have looked through the code and it seems reasonable to me. The fact that the number of parameters is pretty high in many places doesn't matter that much in my opinion as they are at the lower levels of the code.

[coveralls]

Changes Unknown when pulling 4697abc on krka/charsets into * on master*.

[coveralls]

Changes Unknown when pulling 4697abc on krka/charsets into * on master*.

[coveralls]

Changes Unknown when pulling 4697abc on krka/charsets into * on master*.

[coveralls]

Changes Unknown when pulling 4c6a154 on krka/charsets into * on master*.

[josefalcon]

👍

[protocol7]

Not in scope for this PR, but APIs that expose byte[] is a problem. Should we use/do something like ByteString?

[spkrka]

True, though this class is not really directly exposed to regular users of the library.

We could of course try to improve it by making it package private and move all the classes that need it into the same package. Which might make sense anyway.

[protocol7]

Agreed

[spkrka]

Agree to the second statement I presume? :)

[spkrka]

I fixed everything except the public getKey().
Making that method (and some of the others there) not public causes a chain reaction of package changes, I think that should be a separate pull request.

[coveralls]

Changes Unknown when pulling 0d83e48 on krka/charsets into * on master*.

[coveralls]

Changes Unknown when pulling cd7cd94 on krka/charsets into * on master*.

[nresare]

One idea I got reading your discussion is that perhaps it is best to just always use UTF-8. UTF-8 is backwards compatible with ASCII, doesn't encode any characters with null bytes or overlapping ascii control characters. That way the code could probably be simplified while supporting the full range of java Strings.

[spkrka]

Perhaps there is some value in support these charsets though? http://en.wikipedia.org/wiki/ISO/IEC_8859

In any case, I don't think the code complexity is that much worse now, in fact I think some parts are slightly better (key validation and ascii decoder)