Encode form data according to Encoding Object by abcang · Pull Request #1500 · swagger-api/swagger-js

abcang · 2020-03-22T14:50:17Z

Description

Serialize value with respect to Encoding Object when the content-type of the request body is application/x-www-form-urlencoded.
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#encoding-object

Also, OpenAPI v3.1 has a specification to serialize similarly when content-type is multipart/form-data, so it was implemented at the same time.
https://github.com/OAI/OpenAPI-Specification/blob/v3.1.0-dev/versions/3.1.0.md#encodingObject

In addition, a bug that prevented sending an array of files was also fixed.

Motivation and Context

Currently, swagger-client does not work according to the serialization specification of the request body of the form. This pull request is modified to respect the request body serialization settings.
In OpenAPI v3.1, the serialization settings will be respected even when the content-type is multipart/form-data. Although the specification has not yet been officially released, I implemented it before release because I felt it was practical.

Fixes #1470

How Has This Been Tested?

I connected it to swagger-ui and did a simple operation check. Test cases have been added for important cases.

Screenshots (if appropriate):

Types of changes

No code changes (changes to documentation, CI, metadata, etc)
Dependency changes (any modification to dependencies in package.json)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

abcang · 2020-03-22T14:55:16Z

src/execute/oas3/parameter-builders.js

-        }),
-        skipEncoding: true
-      }
+    req.query[parameter.name] = {


Since encoding is not required when sending using FormData, encoding has been changed to execute when executing mergeInQueryOrForm, as in the Open API v2.

abcang · 2020-03-22T14:58:03Z

src/http.js

+// Return value example 4: [['color', 'R,100,G,200,B,150']]
+// Return value example 5: [['R', '100'], ['G', '200'], ['B', '150']]
+// Return value example 6: [['color[R]', '100'], ['color[G]', '200'], ['color[B]', '150']]
+function formatKeyValue(key, input, skipEncoding = false) {


Since the key may change depending on the value, formatValue has been changed to a method that returns a key-value pair

abcang · 2020-03-22T15:02:32Z

test/bugs/ui-4071.js

                industries: {
-                  style: 'form'
+                  style: 'form',
+                  explode: false


If style is form, the default value of explode is true. However, this test seemed to assume the behavior when explode was false, so I set explode to false.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#encoding-object

abcang · 2020-03-22T15:07:59Z

test/bugs/ui-4071.js

        'Content-Type': 'application/x-www-form-urlencoded',
      },
-      body: 'industries=1%2C16'
+      body: 'industries=1,16'


Should , obey allowReserved?

Currently it follows the example table, regardless of allowReserved.
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#style-examples

abcang · 2020-03-22T15:13:21Z

test/oas3/execute/style-explode/query.js

        expect(req).toEqual({
          method: 'GET',
-          url: `/users?id=3 id=4 id=5 id=${SAFE_INPUT_RESULT}`,
+          url: `/users?id=3&id=4&id=5&id=${SAFE_INPUT_RESULT}`,


There is no example when style is spaceDelimited or pipeDelimited and explode is true, but the same behavior as when style is form is probably correct.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#style-examples

abcang · 2020-03-22T15:16:52Z

test/oas3/execute/style-explode/query.js

        expect(req).toEqual({
          method: 'GET',
-          url: `/users?id=3 4 5 ${SAFE_INPUT_RESULT}`,
+          url: `/users?id=3%204%205%20${SAFE_INPUT_RESULT}`,


According to the example table, when style is spaceDelimited, the value is delimited by %20.

https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#style-examples

abcang · 2020-03-22T15:18:03Z

test/oas3/execute/style-explode/query.js

        expect(req).toEqual({
          method: 'GET',
-          url: `/users?id[role]=admin&id[firstName]=Alex&id[greeting]=${SAFE_INPUT_RESULT}`,
+          url: `/users?id%5Brole%5D=admin&id%5BfirstName%5D=Alex&id%5Bgreeting%5D=${SAFE_INPUT_RESULT}`,


encodes [ and ] according to allowReserved

tim-lai · 2020-05-13T23:34:45Z

@abcang Thanks much for this PR. It looks well thought out. A related PR #1527 is inbound that should take care of the multipart case for both OAS2 and OAS3.0 while maintaining the existing urlecoded case. However, I am not sure if it directly address the title's application/x-www-form-urlencoded serialization case. If #1527 does not resolve your issue, please make any updates as needed, and I would be happy to take another look.

abcang · 2020-05-16T17:06:21Z

test/data/sample-multipart-oas3.js

          }
        }
      },
-      '/land/content/uploadImage': {


just fixed the indent

abcang · 2020-05-16T17:28:58Z

test/data/sample-multipart-oas3.js

+              },
+              encoding: {
+                'email[]': {
+                  style: 'form',


I have interpreted that if no Encoding Object is set, explode will not be enabled by default and the format will be based on the default value of contentType.

https://github.com/OAI/OpenAPI-Specification/blob/v3.1.0-dev/versions/3.1.0.md#encoding-object

So I thought I had to explicitly specify explode. (I personally felt that it would be more natural to treat it as explode by default ...)

tim-lai · 2020-05-18T17:36:43Z

@abcang Update: Merged and released #1527 only ensured maintain of existing support of OAS3 x-www-form-urlencoded. After some additional QA, implementation of OAS3 x-www-form-urlencoded itself does need improvement. We should account for explode=true, explode=false, explode (default).

tim-lai

These changes will fix the tests that broke after resolving merge conflicts.

test/http-multipart.js

tim-lai · 2020-05-29T21:33:16Z

@abcang Thanks for the update again! We need one more pass at this, due to the recent dependency reversion/patch of form-data. I've resolved the merge conflicts and requested changes to fix the tests that broke as a result.

test/http-multipart.js

abcang · 2020-05-30T03:59:34Z

Thank you for reviewing and fixing the test!

abcang commented Mar 22, 2020

View reviewed changes

tim-lai self-assigned this May 6, 2020

Encode form data according to Encoding Object

a17f80d

abcang commented May 16, 2020

View reviewed changes

test/data/sample-multipart-oas3.js

}

}

},

'/land/content/uploadImage': {

Copy link
Copy Markdown

Contributor Author

abcang May 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just fixed the indent

abcang commented May 16, 2020

View reviewed changes

tim-lai mentioned this pull request May 18, 2020

Encoding style not respected for requestBody #1470

Closed

frantuma added this to the M2 milestone May 25, 2020

Merge branch 'master' into encode_form_data

9dafd2d

tim-lai suggested changes May 29, 2020

View reviewed changes

test/http-multipart.js Outdated Show resolved Hide resolved

test/http-multipart.js Outdated Show resolved Hide resolved

test/http-multipart.js Outdated Show resolved Hide resolved

Update test/http-multipart.js

f5e9cfd

tim-lai reviewed May 29, 2020

View reviewed changes

test/http-multipart.js Outdated Show resolved Hide resolved

tim-lai added 2 commits May 29, 2020 14:36

Update test/http-multipart.js

63271e8

Update test/http-multipart.js

2af62dd

tim-lai reviewed May 29, 2020

View reviewed changes

test/http-multipart.js Outdated Show resolved Hide resolved

Update test/http-multipart.js

bb107ae

tim-lai approved these changes May 29, 2020

View reviewed changes

tim-lai merged commit 651e857 into swagger-api:master May 29, 2020

tim-lai mentioned this pull request May 29, 2020

fix: Buffer deprecation warning #1551

Merged

10 tasks

abcang deleted the encode_form_data branch May 30, 2020 03:46

hkosova mentioned this pull request Jul 23, 2020

Submitting Array in requestBody with "application/x-www-form-urlencoded" ignores the explode encoding parameter swagger-api/swagger-ui#4836

Closed

phillipuniverse mentioned this pull request Nov 17, 2022

Form request body with array value does not use default 'explode' property #2713

Open

Conversation

abcang commented Mar 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

abcang Mar 22, 2020

Choose a reason for hiding this comment

Uh oh!

abcang Mar 22, 2020

Choose a reason for hiding this comment

Uh oh!

abcang Mar 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abcang Mar 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abcang Mar 22, 2020

Choose a reason for hiding this comment

Uh oh!

abcang Mar 22, 2020

Choose a reason for hiding this comment

Uh oh!

abcang Mar 22, 2020

Choose a reason for hiding this comment

Uh oh!

tim-lai commented May 13, 2020

Uh oh!

abcang May 16, 2020

Choose a reason for hiding this comment

Uh oh!

abcang May 16, 2020

Choose a reason for hiding this comment

Uh oh!

tim-lai commented May 18, 2020

Uh oh!

tim-lai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tim-lai commented May 29, 2020

Uh oh!

Uh oh!

Uh oh!

abcang commented May 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

abcang commented Mar 22, 2020 •

edited

Loading

abcang Mar 22, 2020 •

edited

Loading

abcang Mar 22, 2020 •

edited

Loading