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

Unexpected warning when URL of server object is relative URL #1509

Closed
VcamX opened this issue Dec 8, 2020 · 17 comments
Closed

Unexpected warning when URL of server object is relative URL #1509

VcamX opened this issue Dec 8, 2020 · 17 comments
Assignees

Comments

@VcamX
Copy link

VcamX commented Dec 8, 2020

severs:
  - url: /v1

In SwaggerParseResult, I saw message "attribute .servers. invalid url: /v1 ".

Wonder if relative URL is not supported anymore.

Thanks!

@VcamX
Copy link
Author

VcamX commented Dec 8, 2020

@gracekarina

@VcamX VcamX changed the title Cannot parse relative url of server object Unexpected warning when URL of server object is relative URL Dec 8, 2020
@VcamX
Copy link
Author

VcamX commented Dec 8, 2020

From OpenAPI specs and Swagger doc:

REQUIRED. A URL to the target host. This URL supports Server Variables and MAY be relative, to indicate that the host location is relative to the location where the OpenAPI document is being served. Variable substitutions will be made when a variable is named in {brackets}.

Found the related PR is #1301 and it's introduced since v2.0.18. There's also a change in #1412 for RelativeReferenceTest. It replaced the relative URL with absolute URL.

@r-sreesaran
Copy link
Contributor

The changes has been rolled back.
please do check #1329
Also do try with the latest version of parser.

@VcamX
Copy link
Author

VcamX commented Dec 10, 2020

The changes has been rolled back.
please do check #1329
Also do try with the latest version of parser.

Thanks for your reply!

The issue doesn't seem to be resolved. I tried with latest release v2.0.24 but still saw the warning. A similar issue is also mentioned on #1329 (comment)

I don't see any error/exception but this warning actually. So the point (i think) is if the warning here is appropriate: https://github.com/swagger-api/swagger-parser/blob/master/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/OpenAPIDeserializer.java#L429
Based on OpenAPI specs and Swagger doc, the URL / is valid. And the warning is put into parsing result SwaggerParseResult. This can be misleading imho.

@r-sreesaran
Copy link
Contributor

r-sreesaran commented Dec 10, 2020

As you said having relative url is valid and it should not throw an warning.

@boagm
Copy link

boagm commented Dec 10, 2020

Hi

I think I'm also hitting this issue, when I grab https://petstore3.swagger.io/api/v3/openapi.json which has

servers url | "/api/v3"

and I parse it via

      ParseOptions options = new ParseOptions();
      options.setResolve(true);
      options.setResolveFully(true);
      SwaggerParseResult res = openAPIV3Parser.readLocation(docPath,null, options);

with swagger-parser-v3-2.0.24.jar res.getMessages() shows String index out of range: 0
and res.getOpenAPI() is null.

If I rollback back to swagger-parser-v3-2.0.17.jar, we're all good.

Thanks

@gracekarina
Copy link
Contributor

Hi, @boagm, @VcamX , I have tested with current snapshot of Swagger-Parser, and this is not an issue, I will commit a test to show you.
Thanks for reporting this.

gracekarina added a commit to gracekarina/swagger-parser that referenced this issue Jan 21, 2021
gracekarina added a commit that referenced this issue Jan 21, 2021
@gracekarina gracekarina self-assigned this Jan 21, 2021
@VcamX
Copy link
Author

VcamX commented Jan 22, 2021

@gracekarina Thanks for your reply! Have you tried with local OpenAPI schema file instead of the one is served on the server? In my use case I have to package the schema file in jar.
@boagm wonder if you put the schema file in jar or on a remote location.

@VcamX
Copy link
Author

VcamX commented Jan 22, 2021

I guess the folks here OpenAPITools/openapi-generator#8266 also used local schema file to do codegen

@gracekarina
Copy link
Contributor

Hi @VcamX , Which version of parser is being used?

@VcamX
Copy link
Author

VcamX commented Jan 23, 2021

Hi @VcamX , Which version of parser is being used?

I tried both 2.0.24 and 2.0.18 but they didn't work, but 2.0.17 has no issue as @boagm mentioned.

From OpenAPI specs and Swagger doc:

REQUIRED. A URL to the target host. This URL supports Server Variables and MAY be relative, to indicate that the host location is relative to the location where the OpenAPI document is being served. Variable substitutions will be made when a variable is named in {brackets}.

Found the related PR is #1301 and it's introduced since v2.0.18. There's also a change in #1412 for RelativeReferenceTest. It replaced the relative URL with absolute URL.

I suspect the issue exists since 2.0.18.

@gracekarina
Copy link
Contributor

gracekarina commented Jan 23, 2021

@VcamX, as I have tested locally (with remote and local specs), I could only reproduced the issue in 2.0.18, the following versions does not have the issue. So I kindly ask you to attach here the spec you are using, or a small sample of the jar being use so I can reproduce the issue and help fixed it. Thank you.

@boagm
Copy link

boagm commented Jan 25, 2021

Hi @gracekarina,

Thanks for adding the testcase #1519 , it has enabled me to figure what is the difference that leads to me still hitting the issue with swagger-parser-v3-2.0.24.jar res.getMessages() shows "String index out of range: 0" and "res.getOpenAPI() is null" when parsing the OpenAPI from https://petstore3.swagger.io/api/v3/openapi.json, which has

servers:
   url: "/api/v3"

In your testcase you have
String url = "https://petstore3.swagger.io/api/v3/openapi.json"; So after parsing the server POJ has been modified from the relative/api/v3to becomeServers.url: https://petstore3.swagger.io/api/v3`

While in my case the OpenAPI is already downloaded and on local file system so in I have
String url = "c:\\temp\\petstore3.swagger.io\\api\\v3\\openapi.json";

The spec https://swagger.io/docs/specification/api-host-and-base-path/ says
"If the server URL is relative, it is resolved against the server where the given OpenAPI definition file is hosted"

I note @VcamX said "In my use case I have to package the schema file in jar." I wonder what is passed as arg0 to io.swagger.v3.parser.OpenAPIV3Parser.readLocation() in that case?

So what should "relative" mean for a location on a filesystem?

Interestingly I then tried using:
String url = "file:///c:/temp/petstore3.swagger.io/api/v3/openapi.json";
and found that this actually avoid the "String index out of range: 0" issue, with the latest swagger-parser-v3-2.0.24.jar.

In this case of using this file url the parsed POJ model gives back the unmodified relative path:-
Servers.url: /api/v302

For my use case I could switch to using a file URL rather than a path, but I do think it's rather inconsistent behavour here, so would appropriate you thoughts?

Thanks
Martin

@gracekarina
Copy link
Contributor

gracekarina commented Jan 28, 2021

Hi, @VcamX, @boagm, I added here two tests to try to reproduce the issues here exposed, but I was not able to reproduce them.
As I see it, there are two situations:

  1. In SwaggerParseResult, the message "attribute .servers. invalid url: /v1 ". This validation was removed since 2.0.19 and nexts releases do not show the message.
  2. res.getMessages() shows String index out of range: 0 and res.getOpenAPI() is null. This issue I have not been able to reproduce.

Thanks for your comments. Please check the commits and if you have an spec, or a sample with the issue reproduced please attach it to a new issue and mention this one.

@boagm
Copy link

boagm commented Jan 28, 2021

Thanks @gracekarina,

As per my comment on your "testServerRelativeUrlLocal", we're getting close, but I'd not realized another subtlety .. to reproduce the "String index out of range: 0" we need to be passing an absolute path, just "openapilocal.json" did not reproduce the issue - while "C:\devel\test\openapilocal.json" does.

Thanks
Martin

@VcamX
Copy link
Author

VcamX commented Jan 28, 2021

@gracekarina for my case, I was using relative path like schema/openapi.yml as schema file path.
wonder if it's necessary use the path of schema file to determine the URL of server: https://github.com/swagger-api/swagger-parser/blob/master/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/OpenAPIDeserializer.java#L421

@gracekarina
Copy link
Contributor

gracekarina commented Feb 2, 2021

@VcamX please attach your case here.

Added another test c379a31

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