Utf8 support for the encrypted messages #69
Conversation
| return decodeURIComponent(escape(s)); | ||
| }; | ||
|
|
||
| var str2bin = function(str, array_maker) { |
There was a problem hiding this comment.
Rather than passing in a function to declare an array, any reason not to just pass in an array/Uint8Array result object?
There was a problem hiding this comment.
Hm, I think you may have misunderstood what I was saying.
I whipped this up: http://jsfiddle.net/NtRU9/ that seems like a slightly simpler way to handle it, since we're just passing str.length into the function. Is that a reasonable approach?
Sorry for taking a few days to get back to you.
There was a problem hiding this comment.
Oh, crap you are right of course. You see, I put the actual utf8 conversions into these methods at first - needless to say, it didn't work, since these have nothing to do with parsing text as I had soon discovered. Nevertheless, I kept thinking that we won't know the size of resulting byte array (as would be true in case of utf8 conversion) and thus kept things like this (Uint8Array has a fixed size). I'll rebase things to the newest master and fix it.
|
The native byte arrays do not resize themselves automatically and we do not know the number of resulting bytes in advance. |
the engine now correctly handles any strings containing higher utf codepoints
packets with utf8 usernames
|
Alright, I've fixed these arrays and added utf8 support to the userid packet. I've been checking everything manually so far via the |
utf8 case, made s2k utf8 aware.
|
Alright, I've added automatic tests and it looks like everything's working and ready for a merge. |
|
Oh, and looks like my changes to the master test file fix issue #13 |
| * @param {String} str Any native javascript string | ||
| * @param {String} format | ||
| */ | ||
| this.set_data = function(str, format) { |
There was a problem hiding this comment.
Are the next few helper methods meant to be called outside of the literaldata packet? It seems like from their use we may not want to set them as this.set_data
There was a problem hiding this comment.
Right now there's no point in calling them from outside as write_packet() overrides all internal data anyway. In the future however, I'd like to move away from the read/write packet duo and provide another method that takes the packet's actual contents and works on them rather than throwing it away. Right now, all openpgp_packet_ classes are readonly in essence.
|
I made a couple of small notes, but looking good generally. I'm not usually much of a stickler for commit log but would you mind squashing commits before we merge, a number of the commits change things changed in previous commits and your initial commits were by a "Bob" character which I'm assuming is a local user. Unrelated to your code, but it's annoying that registering the resources/openpgp*js files as binary in gitattributes works with git but github still shows the full diff for those files. Anyone know of a way to fix this? |
| */ | ||
| this.set_data = function(str, format) { | ||
| this.format = format; | ||
| this.data = str |
There was a problem hiding this comment.
I tend to be a proponent of terminating semicolons, and our codebase is generally pretty consistent about using them. I'm not a fan of syntactical religious battles but I think we would generally like to be consistent.
I'm open to other opinions.
Added code to the literal packet implementation - it should correctly convert the native javascript strings into their utf8 counterparts.
To see why it's working look here.