Incorrect URI encoding of links since version 2.0.0

[jimirocks]

Hi, we used version 1 of the library to implement some JSON:API service with slightly extended query parameters. For that reason we are using

JsonApiModelBuilder.jsonApiModel().link("our custom-built self link")

Since the version 1 did not mangle the link URI in JSON in any way we applied URIencoding on our site.

After migration to 2.X.X with spring-boot 3 our links are double uri encoded, however when removing our site encoding, some characters are not encoded properly.

Please consider

1. either make the URI encoding of links optional
2. or to improve the URI encoding to comply

Examples (from our tests with our URIEncoder turned off):

links.self
Expected: http://localhost/test/mockedMutableEntities/mutable123?include=toOneRelation%2CmockedRelations
          got: http://localhost/test/mockedMutableEntities/mutable123?include=toOneRelation,mockedRelations
          
expected: http://localhost/api/entities?filter=description%3D%3D%27descAAA%27
  but was: http://localhost/api/entities?filter=description=='descAAA'
  
links.self
Expected: http://localhost/test/mockedEntities?filter=title%3D%3D%27baf%27%3Bdescription%3Dcontainsic%3D%27haf+haf+haf%27%3Btags%3D%3D%27mocked%27%3BtoOneRelation.id%3D%3D%27toOneRelationId%27%3BmockedRelations.name%3D%3D%27mockedRelationName%27
     got: http://localhost/test/mockedEntities?filter=title=='baf';description=containsic='haf%20haf%20haf';tags=='mocked';toOneRelation.id=='toOneRelationId';mockedRelations.name=='mockedRelationName'

In case you agree, I can also contribute...

[toedter]

Thx for reporting, I'll take a look at it.

[jimirocks]

Hi, thx for quick reaction, do you have any estimation? Or should I just provide some PR with fix triage?

[toedter]

It would help me a lot if you prepare a MR.

[jimirocks]

So what do you prefer? Does it make sense to have the encoding optional/configurable via JsonApiConfiguration ? That's probably the easiest way.

[toedter]

Thx for the PR, I checked it out and found 2 issues that I have to think about:

1. When you set .withLinksUriEncoded(false) in the config, ALL links, even the autogenerated pagination links will be partly unencoded, for example:

{
  "links": {
    "self": "http://localhost:8080/api/movies?fields%5Bmovies%5D=title,year,rating,directors&include=directors&page[number]=1&page[size]=1",
    "first": "http://localhost:8080/api/movies?fields%5Bmovies%5D=title,year,rating,directors&include=directors&page[number]=0&page[size]=1",
    "prev": "http://localhost:8080/api/movies?fields%5Bmovies%5D=title,year,rating,directors&include=directors&page[number]=0&page[size]=1",
    "next": "http://localhost:8080/api/movies?fields%5Bmovies%5D=title,year,rating,directors&include=directors&page[number]=2&page[size]=1",
    "last": "http://localhost:8080/api/movies?fields%5Bmovies%5D=title,year,rating,directors&include=directors&page[number]=249&page[size]=1"
  }
}

Those links would not work with many backends since they would not accept [ and ] in an url-unencoded form. I guess a solution for the library would be to apply the configuration ONLY for the links that are added by the builder, I will take a look.

2. One little thing: most people name it URL encoding not URI encoding, but of course the link hrefs itself are URIs that can be URLs :). So I will think also about the naming...

[toedter]

Would it help solving your problem to url-encode only the link hrefs that are not already encoded. Would a check like below work for you?

public class UrlCheck {
    public static boolean isUrlEncoded(String url) {
        String decodedUrl = URLDecoder.decode(url, StandardCharsets.UTF_8);
        return !url.equals(decodedUrl);
    }

    public static void main(String[] args) {
        String testUrl = "https%3A%2F%2Fexample.com%2Fquery%3Fname%3Dvalue";
        boolean encoded = isUrlEncoded(testUrl);
        System.out.println("Is URL encoded: " + encoded);
    }
}

[peter-plochan]

Hi @toedter, your solution above does not cover a situation when some URL parameter contains a text which just seems to be URL encoded. E.g.

String testUrl = "https://example.com/sketches?title=%2D Model"; // this is quite edge case, but I just wanted to point it out
boolean encoded = isUrlEncoded(testUrl);
System.out.println("Is URL encoded: " + encoded);

// Is URL encoded: true

... or even worse:

String testUrl = "https://example.com/sketches?title=%Another sketch title%";
boolean encoded = isUrlEncoded(testUrl);
System.out.println("Is URL encoded: " + encoded);

// URLDecoder: Illegal hex characters in escape (%) pattern - Error at index 1 in: "An"
// java.lang.IllegalArgumentException: URLDecoder: Illegal hex characters in escape (%) pattern - Error at index 1 in: "An"
//	at java.base/java.net.URLDecoder.decode(URLDecoder.java:237) ...

^ but this can probably be solved by a reasonable exception handling...

[jimirocks]

@toedter I agree with @peter-plochan that auto-detection may lead to buggy behaviour - simply because the url decoding is not idempotent operation on all possible inputs.

I got your point that not uri encoding everything (even by the optionsú is most probably not desired scenario. Therefore I can propose to options how to proceed:

1. change the boolean linksUriEncoded to List<LinkRelation> linksRelationUriDecoded - this would allow to avoid double encoding only some links (handled by specific logic). And it will solve my issue. However may be it's not granular enough.
2. the ultimate approach would be to adjust JsonApiModelBuilder - may be by adding new method for adding plain links and then distinguish it in serializer. - This might lead to some complexities in the library codebase, not sure it is worth it.

I can try to adjust my PR according to one of those proposals - prefer the first one as beeing easier and more pragmatic and also does not prevent introducing the second option later on request. Please tell me 🙏🏻 .

[jimirocks]

Well prepared a PR for 1., please check

[toedter]

Sorry, I was on vacation, will have a look at it soon.

[toedter]

The latest PR looks good to me. Could you please

• update to latest upstream (I hope I could fix the build for PRs)
• add some tests for the new config (I could also do this later but you could provide better test data)

[toedter]

It's in the latest release: https://github.com/toedter/spring-hateoas-jsonapi/releases/tag/v2.1.0