Path Parameters Are Not Encoded

[lew]

What are the steps to reproduce this issue?

1. Create a minimal OpenAPI example like

{
  "info": {
    "title": "My API",
    "version": "1.0.0"
  },
  "openapi": "3.0.0",
  "paths": {
    "/test/{parameter}": {
      "get": {
        "parameters": [
          {
            "in": "path",
            "name": "parameter",
            "required": true,
            "schema": {
              "type": "string"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "OK"
          }
        }
      }
    }
  }
}

2. Generate the functions.
3. Use a parameter with special characters in above example.

What happens?

The parameter parameter is just string concatenated into the URL:

export const getTestParameter = <TData = AxiosResponse<void>>(
  parameter: string,
  options?: AxiosRequestConfig
): Promise<TData> => {
  return axios.get(`/test/${parameter}`, options)
}

As a result, requests fail.

What were you expecting to happen?

Strings in the path fragment must be encoded

export const getTestParameter = <TData = AxiosResponse<void>>(
  parameter: string,
  options?: AxiosRequestConfig
): Promise<TData> => {
  return axios.get(`/test/${encodeURIComponent(parameter)}`, options)
}

Any other comments?

URL encoding should take place here because neither the rest of the app should care about encoding nor is it possible to later encode a fully constructed URL.

A programmer would typically prefer to use a method like this to avoid having to worry about internals:

const someParam = 'special[s]/?test'
const results = useGetTestParameter(someParam)

This bug happens at different places in the generated code.

What versions are you using?

"orval": "6.11.0-alpha.6"

[anymaniax]

@lew I don't think it's a good idea to encode every parameter automatically. Would be easier if you handle it with a mutator no?

[lew]

I believe it is a good idea not to allow the function's user to provide input that breaks the execution.
When a user enters a path segment with a character like /, it completely changes the resulting URI; when they enter a path segment with a reserved character, it can result in invalid HTTP requests.

The RFC 3986 explicitly mentions when to encode the URI:

Under normal circumstances, the only time when octets within a URI
are percent-encoded is during the process of producing the URI from
its component parts. This is when an implementation determines which
of the reserved characters are to be used as subcomponent delimiters
and which can be safely used as data. Once produced, a URI is always
in its percent-encoded form.

In my opinion, this is within orval's code in lines such as

  return axios.get(`/test/${parameter}`, options)

What are the downsides to encoding in this place?

Side note:
The querystring built-in node module transparently translates URI segments, for example:

import querystring from 'querystring'

const qs = querystring.stringify({ param1: 'some&value', param2: 'some other value' })

console.log(qs)
// param1=some%26value&param2=some%20other%20value

[lukaw3d]

Some related docs and issues:

• https://generator.swagger.io/ always encodes slashes
• spec defines allowReserved option for query parameters https://swagger.io/docs/specification/describing-parameters/#allowReserved
• but not for path parameters :/
  • Support reserved chars in path params swagger-api/swagger-js#2597
  • Support for path parameters which can contain slashes OAI/OpenAPI-Specification#892

[anymaniax]

We could add it conditionally with an option in the orval config if it's ok for you

[NimmLor]

I definitely agree that it would be a useful feature to have.
You could make it opt-in for now, and maybe make it the default for the next major release?

[Maxim-Mazurok]

We used to generate API with openapi-generator-cli using typescript-axios template.
It had this:

const localVarPath = `/api/v1/search/{searchTerm}`.replace(`{${"searchTerm"}}`, encodeURIComponent(String(searchTerm)));

Now with orval we got a bug when user tried to search for URL. Would be great to have this option.

Here's my patch:

diff --git a/packages/query/src/index.ts b/packages/query/src/index.ts
index ab7cff8..74a7efc 100644
--- a/packages/query/src/index.ts
+++ b/packages/query/src/index.ts
@@ -37,6 +37,7 @@ import {
   vueMakeRouteReactive,
   vueWrapTypeWithMaybeRef,
   isVue,
+  makeRouteSafe,
 } from './utils';
 
 const AXIOS_DEPENDENCIES: GeneratorDependency[] = [
@@ -234,13 +235,10 @@ const VUE_QUERY_V4_DEPENDENCIES: GeneratorDependency[] = [
     exports: [
       { name: 'unref', values: true },
       { name: 'computed', values: true },
+      { name: 'MaybeRef' },
     ],
     dependency: 'vue',
   },
-  {
-    exports: [{ name: 'MaybeRef' }],
-    dependency: '@tanstack/vue-query/build/lib/types',
-  },
 ];
 
 const isVueQueryV3 = (packageJson: PackageJson | undefined) => {
@@ -298,9 +296,9 @@ const generateQueryRequestFunction = (
   outputClient: OutputClient | OutputClientFunc,
 ) => {
   // Vue - Unwrap path params
-  const route: string = isVue(outputClient)
-    ? vueMakeRouteReactive(_route)
-    : _route;
+  const route: string = makeRouteSafe(
+    isVue(outputClient) ? vueMakeRouteReactive(_route) : _route,
+  );
   const isRequestOptions = override.requestOptions !== false;
   const isFormData = override.formData !== false;
   const isFormUrlEncoded = override.formUrlEncoded !== false;
diff --git a/packages/query/src/utils.ts b/packages/query/src/utils.ts
index 51e5e9a..fe35a5a 100644
--- a/packages/query/src/utils.ts
+++ b/packages/query/src/utils.ts
@@ -98,9 +98,19 @@ export function vueWrapTypeWithMaybeRef(input: string): string {
   return output;
 }
 
+const wrapRouteParameters = (
+  route: string,
+  prepend: string,
+  append: string,
+): string =>
+  (route ?? '').replaceAll(/\${(.+?)}/g, `\${${prepend}$1${append}}`);
+
 // Vue persist reactivity
 export const vueMakeRouteReactive = (route: string): string =>
-  (route ?? '').replaceAll(/\${(\w+)}/g, '${unref($1)}');
+  wrapRouteParameters(route, 'unref(', ')');
+
+export const makeRouteSafe = (route: string): string =>
+  wrapRouteParameters(route, 'encodeURIComponent(String(', '))');
 
 export const isVue = (client: OutputClient | OutputClientFunc) =>
   OutputClient.VUE_QUERY === client;