ferdikoomen / openapi-typescript-codegen

NodeJS library that generates Typescript or Javascript clients based on the OpenAPI specification
MIT License
2.99k stars 525 forks source link

Payload for "application/x-www-form-urlencoded" is actually for "multipart/form-data" #1441

Open soulchild opened 1 year ago

soulchild commented 1 year ago

I have an OpenAPI specification with a contentType of application/x-www-form-urlencoded:

"/foo": {
  "post": {
    "requestBody": {
      "content": {
        "application/x-www-form-urlencoded": {
          "schema": {
            "$ref": "#/someSchema"
          }
        }
      }
    }
  }
}

This library creates POST data using FormData which ends up being suitable for a contentType of multipart/form-data, not application/x-www-form-urlencoded.

In other words, instead of something like:

parameter1=value1&parameter2=value2

it generates a base64-encoded payload, which decodes to:

----------------------------037807260368578567993227
Content-Disposition: form-data; name="parameter1"

value1
--------------

I believe the FormData needs to be passed through URLSearchParams() to generate a payload suitable for application/x-www-form-urlencoded:

const form = new FormData();
form.append('parameter1', 'value1');
form.append('parameter2', 'value2');
console.log(new URLSearchParams(form).toString());

// 'parameter1=value1&parameter2=value2'

Or am I missing something here? Thanks! :smile:

soulchild commented 1 year ago

Ok, it seems like the types for URLSearchParams correctly don't directly accept FormData, so this only works in plain JS:

https://github.com/microsoft/TypeScript/issues/30584

So some other fix is needed.

daankets commented 1 year ago

@soulchild I ran into the same problem today, and found a fix by patching the generated services after generation. In case the mediaType is for url-encoded form data, you can replace: formData: formData by body:new URLSearchParams(formData).toString(). I tested it using the node client.

onurkun commented 1 year ago

You can create new script to fix with a single liner find . -type f -exec sed -i.bkp 's/formData: formData/body: new URLSearchParams(formData as Record<string,any>).toString()/' {} \; && find . -iname '*.bkp' -delete

ttacon commented 1 year ago

I ran into this today as well, from looking at the code, the request templates all need to become contextually aware of the different potential encoding strategies for Forms based on if the Content-Type is either multipart/form-data or application/x-www-form-urlencoded.

As an example, for clients generated to use axios:

There are two potential, simpler, fixes here - (1) either the mediaType earlier, and passing in the optional userHeaders parameter to form.getHeaders(). e.g.:

    const formHeaders = typeof formData?.getHeaders === 'function' && formData?.getHeaders(
        !!options.mediaType ? { 'Content-Type': options.mediaType} : {},
    ) || {}

or (2) alter the order of the object expansion operations:

    const headers = Object.entries({
        Accept: 'application/json',
        ...additionalHeaders,
+       ...formHeaders,
        ...options.headers,
-       ...formHeaders,
    })

Of the above two, the former is likely more resilient due to allowing form-data to control the returned type, the latter is a smaller overall change (although, I'm unsure if the original order of having formHeaders last was intentional).

Would either of these solutions be acceptable as a PR?

jordanshatford commented 8 months ago

Check out our fork of this repository @hey-api/openapi-ts. We have fixed this issue in v0.32.1. If you run into any further issues, open an issue in our repository. Thanks.