openapi: workaround issue importing fully resolved definitions

[megalucio]

Overview

Fix import openapi definitions from files. It fails with big files.

Related Issues

Specify any related issues or pull requests by linking to them.

zaproxy/zaproxy#8193

Checklist

• Update help
• Update changelog
• Run ./gradlew spotlessApply for code formatting
• Write tests
• Check code coverage
• Sign-off commits
• Squash commits
• Use a descriptive title

For more details, please refer to the developer rules and guidelines.

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[megalucio]

recheck

[thc202]

You first need to add the agreeing comment.

[megalucio]

I have read the CLA Document and I hereby sign the CLA

[thc202]

IMO this is not the proper fix, we should change the parser limits instead.

[megalucio]

IMO this is not the proper fix, we should change the parser limits instead.

Agreed, however I understand the limit that is being reached here is the String's Maximum length(Integer.MAX_INT or 2147483647) as seen on the exception

com.fasterxml.jackson.databind.JsonMappingException: TextBuffer overrun: size reached (2147483648)
...
Caused by: java.lang.IllegalStateException: TextBuffer overrun: size reached (2147483648)
...

Not sure how to address that. Besides the method where the exception is being thrown, Json.pretty() belongs to external library swagger-core so it does not look like we can change that.

Another alternative which I was thinking was to catch for the IllegalStateException and only on that instance to get the string directly from the file, which seems a bit more proper.

Thoughts? Any other ideas on how to fix it?

[thc202]

It looks suspicious that's actually hitting the max of the string, what's the size of the original definition? Did you try write the expanded definition to a file to see its size (and contents for that matter)?

I'm aware that's a class/method from the library, but that's "one" line of code.

Agreed that looks better still I'd skip the pretty print instead of reading from the file (as that would break the multi file).

[kingthorin]

@megalucio are you able to answer the latest questions and keep going with this?

[megalucio]

Giving another go to this one, @thc202 what do you mean by the original and expanded definitions? Not sure, I'm following here. Thanks

[thc202]

In SwaggerConverter calls to resolveFully will replace the references with actual values which will make the definition bigger, i was suggesting saving that one and verify the contents and the size.

Would still be interesting to know the size of the original.

[megalucio]

Size of the original is 2.3M

[thc202]

And just before the Json.pretty(…)?

[megalucio]

openApiString.length() = 2378052

[megalucio]

Not sure how to calculate the size there since I have an OpenAPI object

[thc202]

With Json.mapper().writeValueAsString(…) (or Yaml depending what format you have), you can also write to file with one of the other methods of that class.

[megalucio]

It looks suspicious that's actually hitting the max of the string, what's the size of the original definition? Did you try write the expanded definition to a file to see its size (and contents for that matter)?

I'm aware that's a class/method from the library, but that's "one" line of code.

Agreed that looks better still I'd skip the pretty print instead of reading from the file (as that would break the multi file).

Honestly I'm not sure what you mean by the original definition, this is what is being provided. Still unable to process big openapi definitions.

With Json.mapper().writeValueAsString(…) (or Yaml depending what format you have), you can also write to file with one of the other methods of that class.

Yeah, that is where the exception is being thrown, this is what happens inside pretty. But the good thing about using it is that in this instance I can catch the exception and only read directly from file when that happens which respects more the previous implementation, I figure this is more backwards compatible. Only doubt I have is if still use Json.pretty or stick with Json.mapper().writeValueAsString(…)`. I will post a new commit now with this idea see what you think.

[thc202]

Did you try write to file?

[megalucio]

Did you try write to file?

If I am understanding you correctly, I get the same exception when Json.mapper().writeValueAsString so basically cant write to file.

[thc202]

I mean one of the other methods not that one, e.g. writeValue(File, Object).

[megalucio]

Yeah I got a 2.9 GB file 🤦‍♂️

[thc202]

• This needs a spotlessApply.
• Can you add an entry to the changelog? Something like:

### Changed
- Workaround issue loading fully resolved definitions that are too large by trying to use the original definition only (Issue 8193).

[thc202]

This might also need a merge/rebase.

[thc202]

Did you fetch from upstream/zaproxy? (the PR has conflict now)

[megalucio]

I think is ready now, please review again.

[thc202]

I think it still needs a spotlessApply.

[thc202]

Thank you!