`application/x-www-form-urlencoded` encoding in `applyQuery` is incompatible with Apache Olingo - no way to overwrite

[ciprian-dragomir]

Whilst the URLsearchParams offers a clean interface to specify query parameters, it utilises the application/x-www-form-urlencoded encoding underneath (https://url.spec.whatwg.org/#example-constructing-urlsearchparams) and as a result spaces are encoded as "+" and not "%20". This makes the usage incompatible with Apache Olingo who have dropped support for this encoding in 2015.

Spaces especially necessary for the specification of filters. So doing:

const a = new URL('http://foo.com/')
a.searchParams.set('$filter', 'bar eq 3')
a.href

in the browser yields http://foo.com/?%24filter=bar+eq+3 which is not accepted by Olingo (and perhaps other implementations) which follow the spec very strictly.

[janhommes]

Hey @ciprian-dragomir, I see the issue. I would be up for changing it in general to the right encoding. As far as I understood your PR only contains a workaround (setting?). Sorry hadn't much time to look at it in deep.

Do you see any chance that we can change it, without monkey patching fetch?

Thanks Jan

[ciprian-dragomir]

Hi @janhommes Thanks for promptly looking into this!

My PR adds the ability to specify a custom applyQuery function such that devs can overwrite the default behaviour (it does not change the current encoding as we'd have to consider it a breaking change). So we wil still use:

const defaultApplyQuery: ApplyQuery = (url, query) => {
  for (const key in query) {
    if (query.hasOwnProperty(key)) {
      if (url.searchParams.get(key)) {
        url.searchParams.set(key, query[key]);
      } else {
        url.searchParams.append(key, query[key]);
      }
    }
  }

  return url;
};

...
public applyQuery(query: OdataQuery, applyQueryFn = defaultApplyQuery) {
    applyQueryFn(this.url, query);
    return this;
}

by default, but if you specify a custom applyQuery in the ODataConfig, then that will take precedence.

To be clear, there is no monkey patching in the PR, I only mentioned it as this is what a library user has to do to work around the incompatibility as there is no way to intercept the Request.url prior to the fetch provided by the library. It is always the case that applyQuery resets the query params using URL.searchParams which applies the application/x-www-form-urlencoded encoding.

I can change my PR to just use the "standard" encoding (i.e. adhere to spec) since this is recommended. Let me know if you would like me to keep the applyQuery option on ODataConfig - this would probably add more flexibility since not everything adheres to the URL spec I linked to?

[janhommes]

so I would be up for a new major release (maybe 2.0.0) and changing this. It seems like the right approach is to use your mentioned encoding (even if I am atm not that sure how to do this, long time not worked with odata sadly).

Additionally, there are two major bugs that need fixing and better testing which we could include in that release (#127 #126). I guess #126 would also need some breaking changes.

Also, I thought about including https://www.npmjs.com/package/odata-query to solve #98.

If you would submit a PR for the encoding changes, I can craft a 2.0.0-rc release candidate and start working on the other topics (so that you get it fast).

[ciprian-dragomir]

Sure, not a problem, I had already included the logic for this in a test in my earlier PR. This PR should just change the econding mechanism such that it follows the spec. This is mainly encoding spaces in query parameters as "%20" and not "+".

[ciprian-dragomir]

Hi @janhommes , could we have a version 2.0.0-rc npm release please?

[bb94user]

Hi, I am facing the same error, could you please share with me the solution?

[ciprian-dragomir]

Hi, I am facing the same error, could you please share with me the solution?

Hi @bb94user, the solution was submitted in this PR and merged, however it has not been released yet. In the meantime, I've been hijacking window.fetch and modifying the URL just before it gets sent using this logic:

const originalFetch = window.fetch;
window.fetch = (requestInfo, ...rest) => {
  if (typeof requestInfo === 'object') {
    const [path, search] = requestInfo.url.split('?');
    return originalFetch(
      // Replace '+' with '%20' in URL searchParam
      new Request([path, search.replace(/\+/g, '%20')].join('?'), requestInfo),
      ...rest,
    );
  }

  return originalFetch(requestInfo, ...rest);
};

however, this is a dirty hack which applies to trusted URLs which are already built using o.js. The complete solution is in the PR I mentioned earlier.

[janhommes]

sorry for the late response. I wanted to add more features to the new release but failed with time management. I published now a rc to npm with only these changes. Hope that helps you.

[ciprian-dragomir]

sorry for the late response. I wanted to add more features to the new release but failed with time management. I published now a rc to npm with only these changes. Hope that helps you.

Looks good! Many thanks 👍

[bb94user]

Thanks a lot!! @janhommes @ciprian-dragomir 👌🏼