Skip to content

Path parameters are not url encoded #4025

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

Closed
pawanrao opened this issue Dec 12, 2017 · 7 comments
Closed

Path parameters are not url encoded #4025

pawanrao opened this issue Dec 12, 2017 · 7 comments

Comments

@pawanrao
Copy link

Q A
Bug or feature request? Bug
Which Swagger/OpenAPI version? 3.0
Which Swagger-UI version? 3.6.1
How did you install Swagger-UI? Just Serving the dist directory
Which browser & version? Chrome
Which operating system? Windows

Demonstration API definition

openapi: 3.0.0
info:
  title: Test Service
  version: 1.0.0
  description: Swagger test service
servers:
  - url: 'https://testservice.swagger.io/services/docService'
paths:
  '/Docs/{docPath}':
    get:
      summary: Get the document JSON
      tags:
        - Document CRUD
      parameters:
        - in: path
          name: docPath
          description: Path for the document
          required: true
          schema:
            type: string
      responses:
        '200':
          description: OK
        '404':
          description: Document Not Found

Expected Behavior

Reserved URL characters such as /; are not being URL-encoded.
For e.g if docPath above is /home/foobar/baz.txt, the / is not being encoded when I make the request using Try-It-Out->Execute button.
I can replicate this in editor.swagger.io as well.

Current Behavior

Reserved characters should be URL-encoded.
However this works with a Swagger 2.0 definition.

Per my understanding, reserved characters should be url-encoded by default or am I missing anything in my definition?

@shockey
Copy link
Contributor

shockey commented Dec 12, 2017

Per my understanding, reserved characters should be url-encoded by default

Indeed they should - this is a bug in our OpenAPI 3.0 request builder.

@artemyarulin
Copy link

Still an issue in [email protected]

@artemyarulin
Copy link

I can prepare a PR if you can point me where encoding should be done?

@shockey
Copy link
Contributor

shockey commented Jun 20, 2018

Hey @artemyarulin, it might be as simple as enabling escaping here: https://github.com/swagger-api/swagger-js/blob/master/src/execute/oas3/parameter-builders.js#L17. If not, it will be somewhere in the src/execute/oas3 folder 😄

@shockey
Copy link
Contributor

shockey commented Aug 18, 2018

Fixed by swagger-api/swagger-js#1331

@shockey shockey closed this as completed Aug 18, 2018
@rbjorklin
Copy link

rbjorklin commented Sep 7, 2018

Is there any way this could be a toggle? My $DAYJOB provides a SaaS where customers upload a specification of what endpoints should exist and what kind of action that endpoint should execute. It would be very handy to have one single swagger-ui instance all customers could use but that would require / not being escaped in path parameters.

EDIT: We've currently locked our swagger-ui version to 3.18.1 together with this in our spec:

parameters:
  - name: panelPath
    in: path
    description: Panel path
    required: true
    allowReserved: true
    schema:
      type: string

EDIT2: This feels very relevant: https://xkcd.com/1172/

@shockey
Copy link
Contributor

shockey commented Sep 7, 2018

@rbjorklin, in this case we're bound by the specification to not allow this. As a rule, we don't skirt around the spec... from there, it's only a few steps to complete madness: what is a user to do when something works perfectly in Swagger UI but in no other tools, because the functionality is nonstandard?

The best thing you can do is raise an issue over at OpenAPI (https://github.com/OAI/OpenAPI-Specification/issues/new), to get this changed in the spec. Perhaps it'll be included in an upcoming version, and everyone can be happy!

That said, we live in the real world here too... so Swagger UI provides escape hatches you can use to implement this. This requestInterceptor should give you what you want:

{ // this is an imaginary Swagger UI config object... insert requestInterceptor wherever you're calling Swagger UI
  requestInterceptor: (request) => {
    request.url = request.url.replace(/%2F/g, "/")
    return request
  }
}

Just beware that you're going against the standard, and you shouldn't expect your definitions to work correctly in other Swagger/OpenAPI tools unless you customize them as well 😄

@lock lock bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants