Skip to content
This repository has been archived by the owner on Feb 8, 2023. It is now read-only.

JSON generates cookie that it cannot read #17

Open
literakl opened this issue Jun 28, 2018 · 4 comments
Open

JSON generates cookie that it cannot read #17

literakl opened this issue Jun 28, 2018 · 4 comments

Comments

@literakl
Copy link

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
Copy link
Author

literakl commented Jun 28, 2018

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
Copy link
Author

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.

@FagnerMartinsBrack
Copy link
Member

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 :)

tholu pushed a commit to tholu/java-cookie that referenced this issue Feb 28, 2019
- Implement Integration test to verify the issue
@tholu
Copy link
Contributor

tholu commented Feb 28, 2019

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).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants