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

[3.27.4 Regression] Multiple path parameters are not all applied #3508

Closed
kevinoid opened this issue May 9, 2024 · 5 comments · May be fixed by Jircs1/swagger-ui#3
Closed

[3.27.4 Regression] Multiple path parameters are not all applied #3508

kevinoid opened this issue May 9, 2024 · 5 comments · May be fixed by Jircs1/swagger-ui#3

Comments

@kevinoid
Copy link
Contributor

kevinoid commented May 9, 2024

Q&A (please complete the following information)

  • OS: Linux
  • Environment: Node.js v18.19.1
  • Method of installation: npm
  • Swagger-Client version: 3.27.7
  • Swagger/OpenAPI version: Swagger 2.0 or OpenAPI 3.0

Content & configuration

Swagger/OpenAPI definition:

OpenAPI 3.0 example (saved as openapi.yaml)
openapi: 3.0.1
info:
  title: Example OpenAPI 3.0 spec with multiple path parameters
  version: 1.0.0
servers:
- url: https://example.com/
paths:
  /{owner}/{pet}/age:
    get:
      description: Get the age of a given pet for a given owner.
      operationId: getAge
      parameters:
      - name: owner
        in: path
        required: true
        schema:
          type: string
      - name: pet
        in: path
        required: true
        schema:
          type: string
      responses:
        "200":
          description: "Age of the pet, in years"
          content:
            application/json:
              schema:
                type: integer
Swagger 2.0 example (saved as swagger.yaml)
swagger: "2.0"
info:
  version: 1.0.0
  title: Example Swagger 2.0 spec with multiple path parameters
host: example.com
produces:
- application/json
consumes:
- application/json
paths:
  /{owner}/{pet}/age:
    parameters:
    - name: owner
      in: path
      type: string
      required: true
    - name: pet
      in: path
      type: string
      required: true
    get:
      description: Get the age of a given pet for a given owner.
      responses:
        "200":
          description: Age of the pet, in years
          schema:
            type: integer

Describe the bug you're encountering

When an operation contains multiple path parameters, SwaggerClient makes a request to a URL where only one of the path parameters is applied and the other parameter names remain unchanged from the URL template in the request URL.

To reproduce...

Run the following code

import SwaggerClient from 'swagger-client';

function testFetch(url) {
  console.log(`fetch('${url}')`);
  process.exit(url === 'https://example.com/joe/fido/age' ? 0 : 1);
}

const client = await new SwaggerClient({
  spec: {
    swagger: '2.0',
    info: {
      version: '1.0.0',
      title: 'Example Swagger 2.0 spec with multiple path parameters',
    },
    schemes: ['https'],
    host: 'example.com',
    produces: ['application/json'],
    consumes: ['application/json'],
    paths: {
      '/{owner}/{pet}/age': {
        parameters: [
          {
            name: 'owner',
            in: 'path',
            type: 'string',
            required: true,
          },
          {
            name: 'pet',
            in: 'path',
            type: 'string',
            required: true,
          },
        ],
        get: {
          description: 'Get the age of a given pet for a given owner.',
          operationId: 'getAge',
          responses: {
            200: {
              description: 'Age of the pet, in years',
              schema: {
                type: 'integer',
              },
            },
          },
        },
      },
    },
  },
  userFetch: testFetch,
});

await client.apis.default.getAge({
  owner: 'joe',
  pet: 'fido',
});

Expected behavior

The script produces:

fetch('https://example.com/joe/fido/age')

This occurs with swagger-client 3.27.3 and earlier.

Actual behavior

The script produces:

fetch('https://example.com/joe/{pet}/age')

This occurs with swagger-client 3.27.4 and later.

Additional context or thoughts

It appears that this was broken by #3504

@swati410
Copy link

When path has more than one placeholder like tasks/v1/lists/{tasklist}/tasks/{task}/move after replacing first instance swagger-client@3.27.4 is not replacing the other instance.

Code bug here :
pathBuilder is called in loop here with the same pathName tasks/v1/lists/{tasklist}/tasks/{task}/move for each iteration

combinedParameters.forEach((parameter) => {

So after 1st iteration req.url changes to http://tasks/v1/lists/123/tasks/{task}/move
here it is unable to find ``tasks/v1/lists/{tasklist}/tasks/{task}/move in req.url req.url = req.url.replace(pathName, resolvedPathname);`

req.url = req.url.replace(pathName, resolvedPathname);

@swati410
Copy link

@char0n have a look at this PR #3510

@char0n
Copy link
Member

char0n commented May 10, 2024

Hi @swati410,

Thanks for the detailed description of the issue. We're looking at possible vector of remediation + your PR.

@char0n
Copy link
Member

char0n commented May 10, 2024

Option 1

@glowcloud we can return back to using new URL to deconstruct the req.url and use decodeURIComponent for the pathname component. In pathname context, bot new URL and decodeURIComponent are compatible. The downside of this is that we are decoding and encoding already resolved pathname components repeatedly because of

combinedParameters.forEach((parameter) => {
'

Option 2

We will fake path template for openapi-path-templating library

Absolute URLs will be prepended by /. The only side effect is that when contextURL contains {variables} they will be resolved as well.

/https://example.com/1.0.0/execute/oas3/parameter-builders.js?query={param}

Option 3

We will pass baseURL to parameter builds and use it to compute current path template for resolution from req.url. Then we will concatenate the baseURL with resolved path template.


As discussed, we decided to go for Option 3 as it the cleanest and most explicit one.

@char0n
Copy link
Member

char0n commented May 10, 2024

Addressed in #3511

@char0n char0n closed this as completed May 10, 2024
swagger-bot pushed a commit that referenced this issue May 10, 2024
## [3.27.8](v3.27.7...v3.27.8) (2024-05-10)

### Bug Fixes

* **execute:** resolve multiple path parameters with the same name in path templates ([#3511](#3511)) ([f1464a2](f1464a2)), closes [#3508](#3508)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants