2.0.18 regression wrong error message when validating the server url

[jmini]

This example spec:

openapi: 3.0.0
info:
  title: Issue with server
  license:
    name: Apache-2.0
    url: https://www.apache.org/licenses/LICENSE-2.0.html
  version: 1.0.0
servers:
- url: http://{server}.swagger.io:{port}/v2
  description: petstore server
  variables:
    server:
      default: petstore
      enum:
      - petstore
      - qa-petstore
      - dev-petstore
    port:
      default: "80"
      enum:
      - "80"
      - "8080"
- url: https://localhost:8080/{version}
  description: The local server
  variables:
    version:
      default: v2
      enum:
      - v1
      - v2
paths:
  /ping:
    get:
      operationId: pingGet
      responses:
        '201':
          description: OK

With version 2.0.18 this is now reporting errors:

attribute .servers. invalid url : http://{server}.swagger.io:{port}/v2
attribute .servers. invalid url : https://localhost:8080/{version}

With 2.0.17 this was not the case.

---

This can also be reproduced with the example provided in the official spec: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#server-object-example

[jmini]

@jimschubert did some analysis in OpenAPITools/openapi-generator#5413 (comment)

The issue is triggered by this line:

swagger-parser/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/OpenAPIDeserializer.java

Lines 410 to 418 in ee9d1a6

	if(!isValidURL(value) && path != null){
	try {
	final URI absURI = new URI(path);
	if("http".equals(absURI.getScheme()) || "https".equals(absURI.getScheme())){
	value = absURI.resolve(new URI(value)).toString();
	}
	else {
	result.warning(location," invalid url : "+value);
	}

The validate method:

swagger-parser/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/OpenAPIDeserializer.java

Lines 458 to 466 in ee9d1a6

	boolean isValidURL(String urlString){
	try {
	URL url = new URL(urlString);
	url.toURI();
	return true;
	} catch (Exception exception) {
	return false;
	}
	}

Is not considering that the server can contains server-variables.

So adding a warning in the else case as introduced by PR #1301 is not a good idea.

If you would like to validate the server URL properly, you need to resolve the server-variables first.

[r-sreesaran]

I would suggest to revert this change as of now. I will raise a new PR for the original issue 1236

[gracekarina]

Hi, I'm going to review why has this regression happend and work a fix, if indeed we need to reverse we will. Thanks! 👍

[gavl-cdl]

This appears to happen with relative URLs, too. For example:

{
  "openapi": "3.0.2",
  "servers": [
    {
      "url": "/api/v1"
    }
  ]
  [snip]
}

generates: attribute .servers. invalid url : /api/v1

[gracekarina]

Hi thanks, for spotting this I have reverted the changes and added a test to prevent this from happening again. @r-sreesaran about the issue #1236 is it still happening?

[badsyntax]

@gracekarina i've spotted a similar issue on the openapi generator, not sure if it's related: OpenAPITools/openapi-generator#8033