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

parameter builder uses a comma instead of an empty string #1708

Open
encodeering opened this issue Sep 10, 2020 · 3 comments
Open

parameter builder uses a comma instead of an empty string #1708

encodeering opened this issue Sep 10, 2020 · 3 comments

Comments

@encodeering
Copy link

Hello,

it seems that an additional , can appended to the path in some cases and it has been detected using swagger ui's execution feature ([email protected]). It could be traced back to the following line:

req.url = req.url.split(`{${name}}`).join(styledValue);

The default behavior of Array.join will be used if styledValue is undefined.

The following yaml definition creates this case, if the input field in swagger ui remains untouched

paths:
  /openapi/specification{plural}:
    parameters:
    - name: plural
      in: path
      required: false
      schema:
        type: string
        pattern: "s?

Many thanks in advance.

@hkosova
Copy link
Contributor

hkosova commented Sep 14, 2020

This is not a valid OpenAPI definition because path parameters must be required.

If you replace required: false with required: true, does the issue still occur?

@char0n
Copy link
Member

char0n commented Sep 14, 2020

Hi @encodeering,

@hkosova is right about the validity of your definition. Though you're right that this is a possible bug.

Here is a POC demonstrating when the bug occurs:

'/openapi/specification{plural}'.split('{plural}').join(undefined)

As styledValue can become undefined we have to compensate for this case and replace the styledValue with empty string.
Default value of separator for Array.prototype.join is ,. When we pass undefined to the join function it's the same as not passing a parameter at all and , is used to join the values.

@encodeering
Copy link
Author

Hi @hkosova

thanks for pointing this out, wasn't aware of this fact. Going to rewrite the specification as it may suddenly stop working if we update the dependencies.

Cheers

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

3 participants