java-cookie icon indicating copy to clipboard operation
java-cookie copied to clipboard

JSON generates cookie that it cannot read

Open literakl opened this issue 6 years ago • 4 comments

LoginUtils.PageStyleCookie cookie = new LoginUtils.PageStyleCookie(lefLogo, rightLogo, css); cookies.set(LoginUtils.COOKIE_STYLES, cookie); PageStyleCookie stylesCookie = cookies.get(COOKIE_STYLES, PageStyleCookie.class);

Failed to parse JSON of cookie: {"logoLeft":"aa.png"%2C"logoRight":"aa.png"%2C"style":"main.css"}

literakl avatar Jun 28 '18 08:06 literakl

it seems that a cookie parsing is broken, decode does not handle %2C

The bug is in private String decode(String encoded)

  1. it is not efficient because it loops over each match but the first match replaces all occurences with decoded = decoded.replace(encodedChar, decodedChar);
  2. regular expression matches "(%[0-9A-Z]{2})+" multiple subsequent occurences encodedChar=%22%2C%22 and the code will fail there. Remove plus character and it will work.

literakl avatar Jun 28 '18 12:06 literakl

workaround:

static class CustomConverter implements ConverterStrategy {
    @Override
    public String convert(String value, String name) throws ConverterException {
        try {
            return URLDecoder.decode(value, "UTF-8");
        } catch (UnsupportedEncodingException e) {
            return value;
        }
    }
}

Btw I do not understand these converters concept as it is used for decoding only.

literakl avatar Jun 28 '18 13:06 literakl

Hi @literakl, do you want to create a PR with tests fixing the issue you described?

Yes, the converter is currently only working for decoding. If you want, you can create a PR to implement the tests and code for the converter for writing, just like what's done in js-cookie :)

FagnerMartinsBrack avatar Jun 29 '18 01:06 FagnerMartinsBrack

I'm have prepared a PR with an integration test that tests generating and reading simple as well as JSON cookies. I can reproduce the issue, however it is already solved with #21 (this fix is included in the new integration test PR).

tholu avatar Feb 28 '19 14:02 tholu