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

swagger-parser-v3 fails to generate spec on relative server urls on Windows #1553

Closed
jhannes opened this issue Apr 11, 2021 · 7 comments
Closed

Comments

@jhannes
Copy link
Contributor

jhannes commented Apr 11, 2021

After a change in OpenAPIDeserializer, swagger-parser-v3 fails to generate any model for a spec with a relative server url when running on Windows.

The message recorded in result is "begin 0, end -1, length 3", quite unhelpfully, while the result of new OpenAPIParser().readLocation(inputSpec, authorizationValues, options) is just result.openAPI and result.messages both being null

According to the OpenAPI spec relative base paths are supported by the OpenAPI standard. Relative URLs are of course very useful when developing a backend-for-frontend API. At any rate, if swagger-parser was to not support it, users deserve a better error message.

Here is a minimum OpenAPI spec to reproduce the problem:

openapi: 3.0.2
info:
  title: Sample API
  description: A small example to demonstrate individual problems
  version: 0.1.9
servers:
  - url: /v1
    description: Server
paths:
  /pets:
    patch:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Pet'
      responses:
        '200':
          description: Updated
components:
  schemas:
    Pet:
      type: object
      required:
        - pet_type
      properties:
        pet_type:
          type: string
        name:
          type: string
        birth_date:
          type: string
          format: date

As of now, I'm executing the parser through openapi-generator, so I don't have a targeted test case or pull request, but can work on this if you want.

Cause:

Trying to substitute server variable values when there are no variables causes a StringIndexOutOfBoundsException. This line should've checked on value.indexOf("{") before doing substring. Additionally, the code does not support multiple template variables as required by the spec. Please not that the code here uses tabs and spaces inconsistently and looks bad on github.

The substitution only happens on URIParseException, which is caused by Windows-style paths with "" instead of "/".

The exception is caught in OpenAPIDeserializer.deserialize, with result.setMessages(Arrays.asList(e.getMessage()));. I suggest replacing this with result.setMessages(Arrays.asList(e.toString()));, which would have given the slightly more helpful java.lang.StringIndexOutOfBoundsException: begin 0, end -1, length 4 in this case. Or perhaps the exception should be allowed to propagate further?

In my case, the code returns to OpenAPIParser which continues with the next extension (first was OpenAPI, second is Swagger). This means that as OpenAPIParser.readLocation returns even the meager message "begin 0, end -1, length 4" is lost.

Suggested fix

  1. Replace the code for variable checking in io.swagger.v3.parser.util.OpenAPIDeserializer#getServer
  2. Remove catch block in io.swagger.v3.parser.util.OpenAPIDeserializer#deserialize, letting fatal bugs messages propagate to the user instead of causing silent errors. Alternatively, use e.toString() instead of e.getMessage() to get less cryptic errors
  3. If the previous suggestion is not followed, change OpenAPIParser to return immediately if output.getMessages() != null so that errors during processing of OpenAPI aren't overwritten by the Swagger parser
@Klaboe
Copy link

Klaboe commented Apr 15, 2021

Could this be the ultimate cause of OpenAPITools/openapi-generator#8266 mayhaps? My scenario in that bug would make sense if OpenAPIDeserializer.deserialize fails and it then goes on to try deserializing with swagger/openapi2 instead 🤔

@jhannes
Copy link
Contributor Author

jhannes commented Apr 19, 2021

@Klaboe I discovered this issue when I attempted the same upgrade to openapi-generator 5.0.0 as @UnleashSpirit describes, so yes, this is the underlying issue.

@willemsendennis
Copy link

willemsendennis commented Apr 28, 2021

I would love to see this fixed so we can start using openapi generator 5.1.0 which has some fixes we were waiting for. Is someone able to PR this?
What are the contribution policies for this project, I might be able to look into this and create a PR myself... (Or jhannes maybe, since he seems to know the solution ;) )

@jhannes
Copy link
Contributor Author

jhannes commented May 19, 2021

I will be able to make a pull request, but not immediately :-)

@jhannes jhannes changed the title swagger-parser-v3 fails to generate spec on relative server urls swagger-parser-v3 fails to generate spec on relative server urls on Windows May 19, 2021
jhannes added a commit to jhannes-forks/swagger-parser that referenced this issue May 19, 2021
@jhannes
Copy link
Contributor Author

jhannes commented May 19, 2021

@willemsendennis I've submitted a pull request. It looks like I misclassified the core issue - it was resolution of Windows path names

gracekarina added a commit that referenced this issue May 19, 2021
#1553 fails to generate spec on relative server urls on Windows
@jhannes jhannes closed this as completed May 20, 2021
@willemsendennis
Copy link

Thanks @jhannes!
Patiently awaiting the releases ;)

@roasher
Copy link

roasher commented May 25, 2021

+1

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

No branches or pull requests

4 participants