Fix #14: Decoding fails when two encoded characters appear one after another #21
Conversation
…ne after another
|
The integration test with js-cookie fails when I checkout 9e305c9:
Are you getting the same error? |
|
@FagnerMartinsBrack Thanks for checking that, I can reproduce it and need to look into it. |
|
It's been a while here for this PR, wish it could be closed any time soon. |
|
@JevinMenezes If the OP is not continuing to work on this for more than a year then I believe it can be closed. |
|
@JevinMenezes @FagnerMartinsBrack Thanks for the reminder! I'll try to finish it soon. |
| @Test | ||
| public void character_with_2_bytes() { | ||
| Mockito.when( request.getHeader( "cookie" ) ).thenReturn( "c=%C3%A3" ); | ||
| String actual = cookies.get( "c" ); | ||
| String expected = "ã"; | ||
| Assert.assertEquals( expected, actual ); | ||
| } |
There was a problem hiding this comment.
This test case is failing.., (its also failing in Travis CI builds - https://travis-ci.org/github/js-cookie/java-cookie/builds/764105675)
@tholu check this failing test case
There was a problem hiding this comment.
@JevinMenezes Yes, I explicitly added the test case here: 88b8327
Thanks for the reminder! Did not have time yet to fix the issue, will try to get this done over the weekend.
|
@JevinMenezes I fixed the test, but Travis is not running anymore? |
|
@FagnerMartinsBrack Travis builds for this repository have stopped in travis-ci.org and this repo needs to be migrated to travis-ci.com. Can we migrate java-cookie repo to travis-ci.com ? |
|
@JevinMenezes yes what do you want me to do? |
|
Just wanted to ask whether I can approve & request this for organization js-cookie, |
|
@JevinMenezes I received the email and just approved it, let me know if you need anything else |
|
@FagnerMartinsBrack thanks, seems like an admin privilege user can migrate the java-cookie repo and I'm unable to select the repo for migrating to travis-ci.com, would you be able to do it, thanks again , |
|
Done @JevinMenezes |
|
@JevinMenezes Can this be merged now? I'll have to fix conflicts in #23 afterwards. |
| // Decode characters with 3 bytes first, then with 1 byte to fix https://github.com/js-cookie/java-cookie/issues/14 | ||
| return decode( decode( encoded, 3 ), 1 ); | ||
| } | ||
|
|
||
| private String decode( String encoded, Integer bytesPerCharacter ) { | ||
| // Use URLDecoder to fix https://github.com/js-cookie/java-cookie/issues/14 | ||
| String decoded = encoded; | ||
| Pattern pattern = Pattern.compile( "(%[0-9A-Z]{2}){" + bytesPerCharacter + "}" ); | ||
| Matcher matcher = pattern.matcher( encoded ); | ||
| while ( matcher.find() ) { | ||
| String encodedChar = matcher.group(); | ||
| String[] encodedBytes = encodedChar.split( "%" ); | ||
| byte[] bytes = new byte[ encodedBytes.length - 1 ]; | ||
| for ( int i = 1; i < encodedBytes.length; i++ ) { | ||
| String encodedByte = encodedBytes[ i ]; | ||
| bytes[ i - 1 ] = ( byte )Integer.parseInt( encodedByte, 16 ); | ||
| } | ||
| try { | ||
| String decodedChar = new String( bytes, UTF_8 ); | ||
| decoded = decoded.replace( encodedChar, decodedChar ); | ||
| } catch ( UnsupportedEncodingException e ) { |
There was a problem hiding this comment.
@tholu I see that decode implementation with 3 bytes first and then with 1 byte is for fixing #14 , further the issue also mentions #14 (comment) . Will this change in implementation solve #14 (comment) ?
There was a problem hiding this comment.
@tholu seems like my review wasn't submitted a few days ago, apologies for the same. Could you please update on this #21 (comment), I can update the PR post your reply, thanks.
@FagnerMartinsBrack I accidentally did some small refactoring (simplifications) together with the fix, the actual fix for is in Cookies:316-323.
Please review and merge, thanks!
Resolves #14.