StringIndexOutOfBounds exceptions with relative server urls when loading from a file path that includes spaces

[moehkass]

Getting this error when parsing a file:
java.lang.StringIndexOutOfBoundsException: begin 0, end -1, length 25

at java.base/java.lang.String.checkBoundsBeginEnd(String.java:3319)
at java.base/java.lang.String.substring(String.java:1874)
at io.swagger.v3.parser.util.OpenAPIDeserializer.getServer(OpenAPIDeserializer.java:481)
at io.swagger.v3.parser.util.OpenAPIDeserializer.getServersList(OpenAPIDeserializer.java:435)
at io.swagger.v3.parser.util.OpenAPIDeserializer.parseRoot(OpenAPIDeserializer.java:192)
at io.swagger.v3.parser.util.OpenAPIDeserializer.deserialize(OpenAPIDeserializer.java:149)
at io.swagger.v3.parser.OpenAPIV3Parser.parseJsonNode(OpenAPIV3Parser.java:134)
at io.swagger.v3.parser.OpenAPIV3Parser.readContents(OpenAPIV3Parser.java:152)
at io.swagger.v3.parser.OpenAPIV3Parser.readLocation(OpenAPIV3Parser.java:89)
at io.swagger.parser.OpenAPIParser.readLocation(OpenAPIParser.java:16)

after debugging I noticed that the parser is failing to read

servers:
  - url: /just/an/example

but when removing the dash before the url, the parsing goes fine, so he servers part becomes:

servers:
   url: /just/an/example

[jhannes]

This seems related to #1553. I tried to provoke this error when I created the PR for this. What is the context? How is OpenAPIParser.readLocation called? In the #1565 I translated paths going into OpenAPIParser from using Windows \ path separators to /, which fixed the problem in my context.

[moehkass]

I am calling OpenAPIParser in this context:

ParseOptions parseOptions = new ParseOptions();
parseOptions.setResolve(true);
parseOptions.setResolveFully(true);
SwaggerParseResult result = new OpenAPIParser().readLocation(fileLocation,null,parseOptions);
OpenAPI openAPI = result.getOpenAPI();

note that all paths are being parsed and read correctly, the only issue here is in the servers.
Can you please explain how the path separator is related to this issue?

[jhannes]

@mhammadkassem The initial cause of the exception is the URISyntaxException thrown from here: https://github.com/swagger-api/swagger-parser/blob/master/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/OpenAPIDeserializer.java#L476

This is trying to parse your fileLocation variable as a path.

This should've been fixed in 2.0.26. Which version are you using?

[moehkass]

I am using 2.0.26, I just tried to move to version 2.0.27-SNAPSHOT by adding it as a maven dependency in my POM but maven couldn't load it as it is not found on maven central repo.

I thought it's not fixed in the version I'm using. Now that you're saying it is fixed so what is causing this error? knowing that I parsed several files that don't have servers and all worked fine, even this file if I comment the servers part the parsing works fine!

[moehkass]

After debugging it seems that the error was caused by the fileLocation value which includes spaces, so the code stops at this line:
URI absURI = new URI(path.replaceAll("\\\\", "/"));
The error is:
java.net.URISyntaxException: Illegal character in path at index 11: D:/Foldername - withspace/Desktop/xyz/xyz/xyz.yaml and it enters the catch:
String variable = value.substring(value.indexOf("{") + 1, value.indexOf("}")); //value: "/just/an/example"

but still confused why when there is servers, the fileLocation variable is parsed as a path? how is the fileLocation related to servers?

[jhannes]

It's weird if 2.0.26 doesn't have this line: https://github.com/swagger-api/swagger-parser/blob/v2.0.26/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/OpenAPIDeserializer.java#L476

The code for getServer seem to be created with the use case that the file is loaded from an http-location. In that case, resolving server relative to this http-location makes sense. Ideally, the code should not be run when working from a File URL at all.

You should update the title of this issue with "StringIndexOutOfBounds exceptions with relative server urls when loading from a file path with spaces" or something like this

[moehkass]

@jhannes Thank you, I have changed the title.
The line you mentioned is available in the code, I mistakenly mentioned before that it is not (I deleted the comment ), but regardless of that I still find it weird that the the error occurring only when openapi yaml file has relative servers url, and this is due to the getServer function using in its constructer the path variable which is the fileLocation variable in my case referring to the location of the file locally.
this is the function getServer with its constructer:

public Server getServer(ObjectNode obj, String location, OpenAPIDeserializer.ParseResult result, String path)

my doubt is how the path or fileLocation variable related to relative servers!

[jhannes]

@mhammadkassem It's not a good situation, no and it has caused at least two bugs. The reason is that this makes sense if the path is HTTPS (or HTTP) and that the code in getServer is not robust enough. This is the line in question: https://github.com/swagger-api/swagger-parser/blob/v2.0.26/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/OpenAPIDeserializer.java#L477, but the code never get here because new URI is sensitive and https://github.com/swagger-api/swagger-parser/blob/v2.0.26/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/OpenAPIDeserializer.java#L481 doesn't check the return value of String.indexOf for -1.

(This is not my code, I just noticed that it's the same area where I contributed my only fix in this project)

[gracekarina]

Hi @mhammadkassem, thanks for reporting this issue, can you please submit a PR with a failing test that exposes the issue, or a sample project, so we can reproduce.

[HrFlorianHoffmann]

We worked around this issue with the following piece of code:

servers:
  - url: '{host}/my/path/v1'
    variables:
      host:
        default: ''

The empty dummy variable avoids that the statements value.indexOf("{") and value.indexOf("}") fail.

[HrFlorianHoffmann]

Tried to produce a pull request that provides a failing unit test for this issue, but failed.

I looked up the class OpenAPIDeserializerTest and added another unit test that supplies our exact yaml specification as a string to the OpenAPIV3Parser.readContents method. However, we didn't get the expected exception.

I assume that the unit tests call the parser differently than the Maven plug-in. More precisely, I assume that the plugin calls the parser with a file location, such that the input parameter path of the method getServer is filled with a value. In the unit tests, this parameter is null, such that the entry condition path != null skips the affected code.

I tried to force the unit test to supply path with a value, by making the private method readContents in OpenAPIV3Parser public and calling it directly, but this didn't work out, as the location/path variable has lots of other effects, and the code aborted with an unrelated other exception.

Setting up a reproducible sample needs more know-how about the parser's details than I currently have.

[ind1go]

I have created #1893 to expose this issue in a test.

[ind1go]

Fix added to #1893 - it's ready for review by committers.