Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong encoding used for deepObject fields in request bodies #7734

Closed
adriangb opened this issue Jan 5, 2022 · 13 comments
Closed

Wrong encoding used for deepObject fields in request bodies #7734

adriangb opened this issue Jan 5, 2022 · 13 comments

Comments

@adriangb
Copy link

adriangb commented Jan 5, 2022

Example OpenAPI schema:

openapi: 3.0.3
info:
  title: API
  version: 0.1.0
paths:
  /deep:
    post:
      responses:
        '200':
          description: Successful operation
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Outer'
      requestBody:
        content:
          application/x-www-form-urlencoded:
            schema:
              type: object
              properties:
                data:
                  $ref: '#/components/schemas/Outer'
            encoding:
              data:
                style: deepObject
                explode: true
        required: true
components:
  schemas:
    Inner:
      title: Inner
      required:
        - foo
      type: object
      properties:
        foo:
          title: Foo
          type: integer
    Outer:
      title: Outer
      required:
        - inner
        - bar
      type: object
      properties:
        inner:
          $ref: '#/components/schemas/Inner'
        bar:
          title: Bar
          type: number

Input:

{
  "inner": {
    "foo": 0
  },
  "bar": 0
}

curl produced by Swagger UI for the input:

curl -X 'POST' \
  'http://127.0.0.1:8000/deep' \
  -H 'accept: application/json' \
  -H 'Content-Type: application/x-www-form-urlencoded' \
  -d 'data=%7B%0A%20%20%22inner%22%3A%20%7B%0A%20%20%20%20%22foo%22%3A%200%0A%20%20%7D%2C%0A%20%20%22bar%22%3A%200%0A%7D'

It looks like the payload is url-encoded JSON. I believe the correct request should be:

curl -X 'POST' \
  'http://127.0.0.1:8000/deep' \
  -H 'accept: application/json' \
  -H 'Content-Type: application/x-www-form-urlencoded' \
  -d 'data[inner][foo]=0' \
  -d 'data[bar]=0'
@hkosova
Copy link
Contributor

hkosova commented Jan 5, 2022

Your example is actually undefined behavior because deepObject is only defined for simple non-nested objects. The spec does not say what to do with nested objects. There's an open issue in the OpenAPI Specification repository: Support deep objects for query parameters with deepObject style

@adriangb
Copy link
Author

adriangb commented Jan 5, 2022

Hmm the Swagger docs say: "deepObject – a simple way of rendering nested objects using form parameters (applies to objects only)". So yeah it explicitly says "nested objects".

Also, it works just fine for query parameters:

openapi: 3.0.3
info:
  title: API
  version: 0.1.0
paths:
  /deep:
    post:
      parameters:
        - in: query
          name: data
          schema:
            $ref: '#/components/schemas/Outer'
          style: deepObject
          explode: true
      responses:
        '200':
          description: Successful operation
components:
  schemas:
    Inner:
      title: Inner
      required:
        - foo
      type: object
      properties:
        foo:
          title: Foo
          type: integer
    Outer:
      title: Outer
      required:
        - inner
        - bar
      type: object
      properties:
        inner:
          $ref: '#/components/schemas/Inner'
        bar:
          title: Bar
          type: number
curl -X 'POST' \
  'https://editor.swagger.io/deep?data%5Binner%5D[foo]=0&data%5Bbar%5D=0' \
  -H 'accept: application/json'

Which when unquoted is data[inner][foo]=0&data[bar]=0.

I don't know what is right or wrong here, but the messaging and behavior is confusing and inconsistent.

@adriangb
Copy link
Author

adriangb commented Jan 5, 2022

But let's not get sidetracked here. I'll just change my example:

openapi: 3.0.3
info:
  title: API
  version: 0.1.0
paths:
  /deep:
    post:
      responses:
        '200':
          description: Successful operation
      requestBody:
        content:
          application/x-www-form-urlencoded:
            schema:
              type: object
              properties:
                data:
                  $ref: '#/components/schemas/Inner'
            encoding:
              data:
                style: deepObject
                explode: true
        required: true
components:
  schemas:
    Inner:
      title: Inner
      required:
        - foo
      type: object
      properties:
        foo:
          title: Foo
          type: integer
curl -X 'POST' \
  'https://editor.swagger.io/deep' \
  -H 'accept: application/json' \
  -H 'Content-Type: application/x-www-form-urlencoded' \
  -d 'data=%7B%0A%20%20%22inner%22%3A%20%7B%0A%20%20%20%20%22foo%22%3A%200%0A%20%20%7D%2C%0A%20%20%22bar%22%3A%200%0A%7D'

Point still stands.

I'm not sure what's going on in the GitHub UI or if comments get deleted (if you deleted them, I am very very much against that in any context), but you said that typo fixes like this don't get back ported and that it's fixed in the latest spec, so I'm just going to close this issue.

@adriangb adriangb closed this as completed Jan 5, 2022
@adriangb
Copy link
Author

adriangb commented Jan 5, 2022

I am sorry @hkosova . I was the one getting confused. I had two similar tabs open and both conversations were with you: OAI/OpenAPI-Specification#2842 (comment). For a second I thought I was commenting on the other discussion and you had deleted a comment. Little brain malfunction on my end.

If the example in #7734 (comment) is valid, can we re-open this issue?

@adriangb
Copy link
Author

adriangb commented Jan 6, 2022

@hkosova I saw you opened an issue to fix the wording in the OpenAPI spec documentation. Thank you, I think that will go a long way to avoid the confusion in my initial post.

This said, could you clarify if my later example is valid? And if it is, does it represent a bug in Swagger UI? Thanks!

@hkosova
Copy link
Contributor

hkosova commented Jan 11, 2022

Your second definition (wheredata is a non-nested object) looks valid, and its "try it out" behavior looks like a bug.

The generated request is supposed to be

curl -X POST ... -d 'data[foo]=123'
# or
curl -X POST ... -d 'data%5Bfoo%5D=123'

but instead is generated like so (before %-encoding):

... -d 'data={"foo":123}'

The issue here seems to be that the specified encoding is not applied to fields of type: object. Possibly related: #5356.

@adriangb
Copy link
Author

Awesome, thank you for confirming! It does seem related to, but probably not a duplicate of, #5356

@sintro
Copy link

sintro commented Mar 25, 2024

Your second definition (wheredata is a non-nested object) looks valid, and its "try it out" behavior looks like a bug.

The generated request is supposed to be

curl -X POST ... -d 'data[foo]=123'
# or
curl -X POST ... -d 'data%5Bfoo%5D=123'

but instead is generated like so (before %-encoding):

... -d 'data={"foo":123}'

The issue here seems to be that the specified encoding is not applied to fields of type: object. Possibly related: #5356.

Any news? Looks like problem is still here with latest (5.12.0) version of Swagger-UI.
Encodings are ignored for objects in mediaTypes.

glowcloud added a commit to swagger-api/swagger-js that referenced this issue Apr 19, 2024
@glowcloud glowcloud self-assigned this Apr 19, 2024
char0n pushed a commit to swagger-api/swagger-js that referenced this issue Apr 23, 2024
This change is related to Parameter.style="deepObject".

Refs swagger-api/swagger-ui#7734
@char0n
Copy link
Member

char0n commented Apr 23, 2024

Addressed in swagger-api/swagger-js#3474

@POMXARK
Copy link

POMXARK commented May 16, 2024

https://github.com/abbasudo/laravel-purity

This is extremely necessary! I can't convert to format query parameter

filters[is_publish][$contains]=true

Terrible. An outrageous flaw.
And no one has undertaken to fix this with at least a third-party package

@char0n
Copy link
Member

char0n commented May 16, 2024

@POMXARK can you be more specific what the issues is here?

@POMXARK
Copy link

POMXARK commented May 16, 2024

@char0n
filters[is_publish]={"$contains":true}
need filters[is_publish][$contains]=true

                    {
                        "name": "filters",
                        "in": "query",
                        "required": false,
                        "style": "deepObject",
                        "schema": {
                            "properties": {
                                "is_publish": {
                                    "properties": {
                                        "$contains": {
                                            "type": "object",
                                            "example": true
                                        }
                                    },
                                    "type": "object"
                                }
                            },
                            "type": "object"
                        }
                    },

https://github.com/DarkaOnLine/L5-Swagger

     *    @OA\Parameter(
     * *             name="filters",
     * *             in="query",
     * *             required=false,
     *                 style="deepObject",
     * *             @OA\Schema(
     * *                 type="object",
     * *                 @OA\Property(property="is_publish", type="object",
     *                   @OA\Property(property="$contains", type="object", example=true)
     *              ),
     * *             )
     * *         ),

@char0n
Copy link
Member

char0n commented May 16, 2024

@POMXARK as mentioned in #7734 (comment), the OpenAPI spec doesn't specify what to do with nested values. Given that the behavior is undefined by the spec, we opted for implementing it as standard JSON serialization format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants