devowlio / wp-react-starter

🚀WordPress Plugin Boilerplate using modern web techs like TypeScript, SASS, and so on... on top of a local development environment with Docker and predefined GitLab CI for continous integration and deployment!
https://devowl.io/wp-react-starter/
Other
387 stars 51 forks source link

commonUrlBuilder double url encoding of route params #30

Closed serasset closed 3 years ago

serasset commented 3 years ago

As the template is missing an example of a API call with parameters, I may be using it in an incorrect way.

I created an endpoint in php expecting a query parameter. The Typescript declaration is the foillowing:

import {
    RouteLocationInterface,
    RouteHttpVerb,
    RouteResponseInterface,
    RouteRequestInterface,
    RouteParamsInterface
} from "@dbnary-dashboard/utils";

export const sparqlGetLocation: RouteLocationInterface = {
    path: "/sparql",
    method: RouteHttpVerb.GET
};
export type SparqlRequest = RouteRequestInterface;
export type SparqlParams = RouteParamsInterface & { query: string };

And I use it with :

async function doSparqlQuery(event: React.MouseEvent) {
    event.persist();
    const result = await request<SparqlRequest, SparqlParams, SparqlResponse>({
        location: sparqlGetLocation,
        params: { query: "SELECT * FROM { ?s ?p ?o } LIMIT 10" }
    });
    const usedUrl = urlBuilder({ location: sparqlGetLocation });
    alert(`${usedUrl}\n\n${JSON.stringify(result, undefined, 4)}`);
    event.preventDefault();
}

The code does not work as expected as it leads to the queried URL :

http://localhost:8080/wp-json/dbnary-dashboard/v1/sparql?_v=1.0.0&query=SELECT%2520*%2520FROM%2520%257B%2520%253Fs%2520%253Fp%2520%253Fo%2520%257D%2520LIMIT%252010

(see how the query parameter is double encoded (%2520 instead of %20).

I tracked down the problem to packages/utils/lib/factory/ajax/commonUrlBuilder.tsx where the parameters are explicitely encoded at:

    // Find undeclared body params (which are not bind above) and add it to GET query
    for (const checkParam of Object.keys(params)) {
        if (foundParams.indexOf(checkParam) === -1) {
            getParams[checkParam] = encodeURIComponent((params as any)[checkParam]);
        }
    }

And afterward, the call to Url.toString() at return apiUrl.set("query", deepMerge.all([options.restQuery, getParams, query])).toString();will re-encode the parameter (using stringify).

Why are parameters URIencoded explicitely in commonUrlBuilder ?

Am I using the boilerplate code incorrectly there ?

matzeeable commented 3 years ago

Hey @serasset !

Thanks for your issue. I validated and fixed the issue, see commit above.

Regards, Matthew 😊