swagger-parser-v3 can throw NPE due to non-initialized instance fields.

[dhoffer]

swagger-parser-v3 can throw NPE parsing/loading Open API spec. The NPE is caused by io.swagger.v3.parser.util#ResolverFully because the schemas and examples fields are not initialized in all cases.

Version 2.0.0-rc3.

Our local fix was to just always initialize all the private fields maps with empty HashMap.

[webron]

It would help to get an API definition to reproduce it.

[dhoffer]

I have attached a sample but you can see in that class that only 1 of the instance fields is initialized in some cases.

Note this parser seems incomplete in may ways as we have found several bugs. Is there a better Open API v3 parser that we should be using? I am not using this component directly but through another component that has this as a dependency. Should they be using a different more complete parser? I also noted this is an RC version when will this be released as final non-RC?

openapi_sample.zip

[gracekarina]

Hi it is possible some of this issues have been solved in the work done in parser after rc3, can you please try with the snapshot?. Thanks!

[dhoffer]

What is the version of the new snapshot, I'm not finding it.

[gracekarina]

2.0.0-SNAPSHOT.

[gracekarina]

can you please send this spec: './resources/urn_jsonschema_com_issinc_odin_display_CreatePlan.json'

[dhoffer]

I have attached the requested file.

On Wed, Apr 11, 2018 at 6:55 AM, Grace Karina Gonzalez Diaz < notifications@github.com> wrote:

can you please send this spec: './resources/urn_jsonschema_com_issinc_odin_display_CreatePlan.json'

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/swagger-api/swagger-parser/issues/682#issuecomment-380441471, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfwr_gh2Scpz1xKlwU8eusQDdvqcokZks5tnf0ogaJpZM4TPLo1 .

[gracekarina]

this is part of the file: and it calls the route : './resources/urn_jsonschema_com_issinc_odin_display_CreatePlan.json',

paths:
  /JTasker/startRun:
    post:
      tags:
        - JTasker
      summary: Start JTasker Run
      operationId: JTasker_startRun
      requestBody:
        content:
          application/json:
            schema:
              $ref: './resources/urn_jsonschema_com_issinc_odin_display_CreatePlan.json'
        required: true
      responses:
        '200':
          description: Success
          content:
            application/json:
              schema:
                type: array
                items:
                  type: string
        '500':
          description: Server Error

[gracekarina]

by the way the ref it's built I assume that ./resources/urn_jsonschema_com_issinc_odin_display_CreatePlan.json' it has just schemas inside.

[dhoffer]

Yes that is correct. It is the schema for that requestBody.

[dhoffer]

I have tried with 2.0.0-SNAPSHOT and it too cannot load from RELATIVE ref.

Unable to load RELATIVE ref: ./resources/urnjsonschema com_issinc_odin_display_CreatePlan.json

On Wed, Apr 11, 2018 at 7:19 AM, David Hoffer dhoffer6@gmail.com wrote:

I have attached the requested file.

On Wed, Apr 11, 2018 at 6:55 AM, Grace Karina Gonzalez Diaz < notifications@github.com> wrote:

can you please send this spec: './resources/urn_jsonschema_com_issinc_odin_display_CreatePlan.json'

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/swagger-api/swagger-parser/issues/682#issuecomment-380441471, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfwr_gh2Scpz1xKlwU8eusQDdvqcokZks5tnf0ogaJpZM4TPLo1 .

[gracekarina]

yes, so I need that file, the one with the schema. or is it the same? I see the file is a json and you sent us a yaml

[dhoffer]

Yes I already attached that file.

On Wed, Apr 11, 2018 at 7:30 AM, Grace Karina Gonzalez Diaz < notifications@github.com> wrote:

yes, so I need that file, the one with the schema. or is it the same?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/swagger-api/swagger-parser/issues/682#issuecomment-380452300, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfwryGjuzf9IG9Wug2nZ6ofZLRQ0wppks5tngWGgaJpZM4TPLo1 .

[gracekarina]

yess, sorry! I'll check again

[gracekarina]

Hi!. In 2.0.0-SNAPSHOT the test passes this is the output after parsing with resolveFully:

  tags:
  - JTasker
  summary: Start JTasker Run
  operationId: JTasker_startRun
  requestBody:
    content:
      application/json:
        schema:
          type: object
          properties:
            createPlanEvents:
              type: object
              properties:
                name:
                  type: string
                description:
                  type: string
                events:
                  type: array
                  items:
                    type: string
            createPlanSensors:
              type: object
              properties:
                sensors:
                  type: array
                  items:
                    type: string
    required: true
  responses:
    200:
      description: Success
      content:
        application/json:
          schema:
            type: array
            items:
              type: string
    500:
      description: Server Error

[gracekarina]

this is the test

 @Test
    public void issues() throws Exception {
        OpenAPIV3Parser parser = new OpenAPIV3Parser();
        ParseOptions options = new ParseOptions();
        options.setResolveFully(true);
        final SwaggerParseResult result = parser.readLocation("src/test/resources/odin.yaml", null, options);
        Assert.assertNotNull(result.getOpenAPI());

        Yaml.prettyPrint(result.getOpenAPI().getPaths().get("/JTasker/startRun"));
        Assert.assertTrue(result.getMessages().isEmpty());

    }

[dhoffer]

That one does not use any Relative refs. Try one that uses $ref:

On Wed, Apr 11, 2018 at 7:39 AM, Grace Karina Gonzalez Diaz < notifications@github.com> wrote:

this is the test

@Test public void issues() throws Exception { OpenAPIV3Parser parser = new OpenAPIV3Parser(); ParseOptions options = new ParseOptions(); options.setResolveFully(true); final SwaggerParseResult result = parser.readLocation("src/test/resources/odin.yaml", null, options); Assert.assertNotNull(result.getOpenAPI());

    Yaml.prettyPrint(result.getOpenAPI().getPaths().get("/JTasker/startRun"));
    Assert.assertTrue(result.getMessages().isEmpty());

}

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/swagger-api/swagger-parser/issues/682#issuecomment-380455043, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfwr_qqLmU5fFvuI2EjDndT0avQA_iXks5tngd0gaJpZM4TPLo1 .

[gracekarina]

it does:

paths:
  /JTasker/startRun:
    post:
      tags:
        - JTasker
      summary: Start JTasker Run
      operationId: JTasker_startRun
      requestBody:
        content:
          application/json:
            schema:
              $ref: './resources/urn_jsonschema_com_issinc_odin_display_CreatePlan.json'
        required: true
      responses:
        '200':
          description: Success
          content:
            application/json:
              schema:
                type: array
                items:
                  type: string
        '500':
          description: Server Error

[dhoffer]

Okay I see that one does. Let me see how you are configuring the ParseOptions. I will see if this is different. Note I am not using this directly but indirectly with other 3rd party component. I will have to check how they are configuring your parser.

[dhoffer]

The prior configuration of ParseOptions was like this: ParseOptions options = new ParseOptions(); options.setResolve(true); options.setResolveCombinators(false); options.setResolveFully(true);

I have changed their configuration to be like yours (only setResolveFully is set true) but it still fails with this error: java.lang.RuntimeException: Could not find ./resources/urn_jsonschema_com_issinc_odin_display_CreatePlan.json on the classpath

I think there is a problem with your test setup. Make sure the relative resources are in subfolder to spec file. E.g. make sure your tests have this directory structure: webroot/odin.yaml webroot/resources/urn_jsonschema_com_issinc_odin_display_CreatePlan.json

Also when running tests in Maven you would normally load resources like this: getClass().getResource(/webroot/odin.yaml) as by definition everything in src/test/resources is in the test classpath. If you add 'src/test/resources/' prefix then that is not a valid classpath value.

Note our use case is to load the spec file like this: parser.readLocation("/webroot/odin-ui-openapi-v3.yml", null, options);

And we are loading '/webroot/odin-ui-openapi-v3.yml' from the classpath.

[dhoffer]

Yeah I have tracked this down some more. If I convert my valid classpath spec resource to a file URI then it works. So the problem appears to be that the classpath loading in the parser is completely broken. As I mentioned above your test case would not be a valid classpath test case.

[gracekarina]

@dhoffer, Please try this sample, it is done with the files you sent. it is successful parser-cp-loader.zip

[dhoffer]

You are using a completely different parser artifact.

io.swagger.parser.v3 swagger-parser 2.0.0-SNAPSHOT

Here is the one I am using and have been discussing. Which is the right one?

io.swagger.parser.v3 swagger-parser-v3 2.0.0-SNAPSHOT

On Wed, Apr 11, 2018 at 9:12 AM, Grace Karina Gonzalez Diaz < notifications@github.com> wrote:

@dhoffer https://github.com/dhoffer, Please try this sample, it is done with the files you sent. it is successful parser-cp-loader.zip https://github.com/swagger-api/swagger-parser/files/1899391/parser-cp-loader.zip

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/swagger-api/swagger-parser/issues/682#issuecomment-380488280, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfwr2RnFB9iIWK6XjU-CA2jZ2yFOksGks5tnh1LgaJpZM4TPLo1 .

[gracekarina]

we'll inflector's 2.0 is using: https://github.com/swagger-api/swagger-inflector/blob/2.0/pom.xml#L300-L304

            <groupId>io.swagger.parser.v3</groupId>
            <artifactId>swagger-parser</artifactId>
            <version>${swagger-parser-version}</version>
        </dependency>

[dhoffer]

Why do you have two that do the same thing? It's not simple to switch this as its a transitive dependency on another component. I can try to do that but...again why do you have multiple(s) that do the same thing? Which one is more current?

On Wed, Apr 11, 2018 at 9:34 AM, Grace Karina Gonzalez Diaz < notifications@github.com> wrote:

we'll inflector's 2.0 is using:

        <groupId>io.swagger.parser.v3</groupId>
        <artifactId>swagger-parser</artifactId>
        <version>${swagger-parser-version}</version>
    </dependency>````

And codegen uses both, why don`t you try with the one I sent?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/swagger-api/swagger-parser/issues/682#issuecomment-380496563, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfwr7khNW2smATVvduYjxoCDcwQtL7rks5tniKfgaJpZM4TPLo1 .

[dhoffer]

Okay it turns out your new artifactId is just really a parent that has the other as a dependency (plus others).

However your project fails to build.

unable to read location /C:/svn/parser-cp-loader/target/classes/spec/ofSpec/odin-ui-openapi-v3.yaml

which results in a NULL openAPI which then results in NPE.

You might want to try your tests on a non Mac system and see how it goes.

-Dave

On Wed, Apr 11, 2018 at 9:39 AM, David Hoffer dhoffer6@gmail.com wrote:

Why do you have two that do the same thing? It's not simple to switch this as its a transitive dependency on another component. I can try to do that but...again why do you have multiple(s) that do the same thing? Which one is more current?

On Wed, Apr 11, 2018 at 9:34 AM, Grace Karina Gonzalez Diaz < notifications@github.com> wrote:

we'll inflector's 2.0 is using:

        <groupId>io.swagger.parser.v3</groupId>
        <artifactId>swagger-parser</artifactId>
        <version>${swagger-parser-version}</version>
    </dependency>````

And codegen uses both, why don`t you try with the one I sent?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/swagger-api/swagger-parser/issues/682#issuecomment-380496563, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfwr7khNW2smATVvduYjxoCDcwQtL7rks5tniKfgaJpZM4TPLo1 .

[gracekarina]

where are you trying? which OS?

[gracekarina]

swagger-parser and swagger-parser-v3 are modules of the same project and share lifecycle, 2.0.0-SNAPSHOT is the latest version; swagger-parser also supports v2 spec, for the rest is equivalent to swagger-parser-v3 which uses internally for 3.0 spec

[dhoffer]

We generally develop on Windows but runtime will be Windows and Linux.

On Wed, Apr 11, 2018 at 9:53 AM, Grace Karina Gonzalez Diaz < notifications@github.com> wrote:

where are you trying? which OS?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/swagger-api/swagger-parser/issues/682#issuecomment-380502972, or mute the thread https://github.com/notifications/unsubscribe-auth/ABfwr9gkkHPqJIjYISfPRtPJv5G0q6HKks5tnibjgaJpZM4TPLo1 .

[gracekarina]

ok, let me check another OS.

[gracekarina]

parser-cp-loader.zip Hi @dhoffer sending sample, tested succesfully on windows. Please let us know if this is still an issue. Also added a test with the spec provided to parser. Regards.

[dhoffer]

You are not testing the classpath usecase with your test sample you just sent. Look at your code. In the Controller you have this: String LOCATION = getClass().getResource("/spec/ofSpec/odin-ui-openapi-v3.yaml").toString();

That converts a classpath value to a file URI. Look at the value of LOCATION in your debugger. Also you can see when you step through the code you are loading that as a file not from class path.

Change that string to be the following to keep it a valid class path value. String LOCATION = "/spec/ofSpec/odin-ui-openapi-v3.yaml";

And you will see the problem. Your class path code in this artifact is completely broken. You have to properly configure your tests so you are testing the case that does not work.

[gracekarina]

this comment you made before:

Also when running tests in Maven you would normally load resources like this:
getClass().getResource(/webroot/odin.yaml)

[gracekarina]

The idea is that you change the artifactId in your project and let us know how that goes. I sent the sample in order to show you the proper way to setup parser. Also please check the test in parser I added.

[dhoffer]

Yes as I mentioned above your test is not valid for class path resource. getClass().getResource() & getClass().getResourceAsStream() are the way you load class path resources in Java. Since your API accepts any location value (http/file/classpath) those go in the parser not outside in your tests. Obviously if you convert a classpath value to file outside the parser you are not testing the classpath use case anymore.

[webron]

@dhoffer you've mentioned that you're not using the library directly but through another component that depends on it. Which one is it? Can you provide a link to it?

[dhoffer]

Sorry been swamped, just getting back to this. I have already worked with the other library provider and tracked the issue down to this component so that is not relevant for the issues I have created here.

It turns out my 'workaround' to always use file URI path doesn't work if the file/resource is located in a jar so I had to finally fix your classpath loading issues (which was one if the issues I created). I forked (copied) your latest snapshot and found that the unit test that @gracekarina added is completely wrong. That test must fail when you fix the problem so that is obviously an anti-unit-test.

I do have a workaround that fixes the classpath issue but its not ready for general production so that is our local fix. The problem is there is no much bad code (as well as bad tests) that a significant part of the library would need to be fixed to properly resolve this. I just added some workarounds to properly handle classpath loading with relative resources.

[gracekarina]

@dhoffer Hi, I'm currently working to fix the issue of classpath, including when the file it's on a jar file. Thanks!.

[webron]

@dhoffer while we appreciate the feedback, there are proper ways to deliver it. Throwing around insults doesn't help the library improve, and doesn't help in providing you with a higher quality of a product.

If you'd like to help the project improve, we'd appreciate filing specific tickets on issues you've seen, and we'll gladly look into them and fix whatever is possible. Like our other open source projects, and open source projects in general, we depend a lot on the community to provide us with the problems they encounter so we can address them. We appreciate fixes as well, but also understand it's time consuming and can't always be provided.

However, your phrasing throughout this thread is very unfavorable, and while I refrained from addressing it until now, it's time to voice it.

I bring this up not to rattle any cages, but rather in hopes that we can minimize the negativity and work together to get a better product that would benefit all.

[gracekarina]

fixed by #1134 issue related to issue #1133