A tale of fixing a tiny OpenAPI bug

TL;DR : I found and fixed a tiny bug in OpenAPI, learnt about the library, jackson and git in the process.

Yesterday, I was sharing some tips on how to get started with OpenAPI generators. Turns out, within minutes of me starting to use the library, I found a bug that was causing heap space exceptions. This blog is about what I learnt while fixing it 😊.

Facing the bug

The first thing I started doing while testing the application, before even creating my own generator was running a few existing generators for languages I know. I started with Kotlin, and added the debug flags to it to see what kind of objects were generated by the OpenAPI parser.

Within minutes, I was running the Kotlin (of course) generator, with the debugSupportingFiles global property activated.

After a few seconds, the process crashed with a Java heap space exception:

[main] INFO  o.o.codegen.TemplateManager - writing file /Users/julienlengrand-lambert/Developer/openapi-generator/samples/client/petstore/kotlin/http/client/docs/UserApi.md
[main] INFO  o.o.codegen.DefaultGenerator - ############ Supporting file info ############
Exception in thread "main" java.lang.OutOfMemoryError: Java heap space
	at com.fasterxml.jackson.core.util.TextBuffer.carr(TextBuffer.java:967)
	at com.fasterxml.jackson.core.util.TextBuffer.expand(TextBuffer.java:928)
	at com.fasterxml.jackson.core.util.TextBuffer.append(TextBuffer.java:682)
	at com.fasterxml.jackson.core.io.SegmentedStringWriter.write(SegmentedStringWriter.java:58)
	at com.fasterxml.jackson.core.json.WriterBasedJsonGenerator.writeRaw(WriterBasedJsonGenerator.java:605)
	at com.fasterxml.jackson.core.util.DefaultIndenter.writeIndentation(DefaultIndenter.java:94)
	...

Not knowing at all what I was doing, I asked for directions on the Slack channel of the project and got suggestions within minutes with ideas of things to try out.

For me, the main question was : Is it only happening on my machine 😬?

Justin was nice enough to file a bug for me, and I started diving into the problem.

Finding the culprit

Turns out, the issue only happens in debug mode, with that specific flag set. It was also happening for all of the generators I tried, and all the specification files. Extending the heap size to humongous values wasn't helping. It had to come from the code somehow.

After a little while, I could track down the line that was creating the crash. It was all happening in the DefaultGenerator (which all generators extend from), and only if the debug flag was set. Makes sense.

if (GlobalSettings.getProperty("debugSupportingFiles") != null) {
	LOGGER.info("############ Supporting file info ############");
	Json.prettyPrint(bundle);  // BIG BOUM, BIG BADABOUM 
}

Jason, again, very helpful on Slack, mentioned that the objects generated by the generators can contain infinite loops. Those loops, when trying to transform them into a JSON file would crash the JVM. That's also what was showing in the stack trace of the exception.

It was time to track down when the bug was introduced!

Git bisect to the rescue!

I won't go deep into how git bisect works. You can read about it yourself. The basic idea is that you provide git with a valid commit (before the bug was there) and a faulty commit (with the bug). You'll also need a quick way to see if the bug is present (in my case, running the run configuration and seeing the heap space crash).

Git will iteratively select commits and ask you to tell him whether the bug is there, until you know precisely the ONE COMMIT which introduced the problem.

It basically looked like this :

$ git checkout master # clean state
$ git bisect start
$ git bisect bad HEAD # bug in there
$ git bisect good v5.0.0 # no bug
$ git bisect good # no crash
$ git bisect bad # crash
$ git bisect good
$ git bisect bad
$ git bisect good
$ git bisect bad
$ git bisect good
$ git bisect end
$ git bisect reset # back to work, let's check the bug now

Honestly, I never had to use it in the past in my professional life and I feel blessed to have an extra tool in my box now 😊. The whole thing was sorted in less than 5 minutes 😅.

Understanding the bug

Once we know the commit which introduced the issue, it's easy to track down where exactly the problem is. Look, it's right there :

    /**
     * @return contentTypeToOperation
     * returns a map where the key is the request body content type and the value is the current CodegenOperation
     * this is needed by templates when a different signature is needed for each request body content type
     */
    public Map<String, CodegenOperation> getContentTypeToOperation() {
        LinkedHashMap<String, CodegenOperation> contentTypeToOperation = new LinkedHashMap<>();
        if (bodyParam == null) {
            return null;
        }
        LinkedHashMap<String, CodegenMediaType> content = bodyParam.getContent();
        for (String contentType: content.keySet()) {
            contentTypeToOperation.put(contentType, this);
        }
        return contentTypeToOperation;
    }

Once you understand what the method does, it's quite clear why the heap explodes. This method is basically adding to an object, the object itself, but with a key of a certain type of format. No wonder it generates infinite loops when the JSON parser runs through it 😊.

Now, the actual problem is not that method itself. The real issue is that it's not called anywhere 😅. It shouldn't be run at all. Actually, there's literally 0 direct usages of the method in the whole library (even in the toString method)

Obviously, the method is used. But only in the Python template, where it is actually working as expected.

class BaseApi(api_client.Api):
{{#if bodyParam}}
    {{#each getContentTypeToOperation}} // Our method here
    {{> endpoint_args_baseapi_wrapper contentType=@key this=this}}

    {{/each}}
    {{> endpoint_args_baseapi_wrapper contentType="null" this=this}}

{{else}}
    @typing.overload
    def _{{operationId}}_oapg(
    {{> endpoint_args isOverload=true skipDeserialization="False" contentType="null"}}
{{/if}}

Still, placing a breakpoint makes it clear enough that Jackson DOES enter the method when converting the object to JSON.

I tried upgrading Jackson and other shenanigans, until one of my colleagues made it all obvious : "Would your method start with get or set by any chance?".

Finally, it all made sense : Jackson uses reflection to convert the object into JSON representation. Somehow, it takes the class and goes through all of the getters and setters iteratively. The method was assumed to be a getter, hence creating the crash.

Fixing the bug

As usualy with software, once we know what the problem is it's actually quite easy to fix it. And even then, I went through 2 iterations.

My first thought was to add the @JsonIgnore annotation to the method, to tell Jackson to ignore it. It worked.

    @JsonIgnore // The magic line
    public Map<String, CodegenOperation> getContentTypeToOperation() {
        LinkedHashMap<String, CodegenOperation> contentTypeToOperation = new LinkedHashMap<>();
        if (bodyParam == null) {
            return null;
        }
        LinkedHashMap<String, CodegenMediaType> content = bodyParam.getContent();
        for (String contentType: content.keySet()) {
            contentTypeToOperation.put(contentType, this);
        }
        return contentTypeToOperation;
    }

I didn't quite like it though. It put some direct dependency to Jackson in a place there wasn't before, and there was no clear explanation either. The me from the future would have hated me for doing this.

The second attempt was, I think, better and that's the one which got merged : Rename the method to remove the get 😅.

public Map<String, CodegenOperation> contentTypeToOperation() {
        LinkedHashMap<String, CodegenOperation> contentTypeToOperation = new LinkedHashMap<>();
        if (bodyParam == null) {
            return null;
        }
        LinkedHashMap<String, CodegenMediaType> content = bodyParam.getContent();
        for (String contentType: content.keySet()) {
            contentTypeToOperation.put(contentType, this);
        }
        return contentTypeToOperation;
    }

William was nice enough to add a test to test for future regression. I wasn't sure how to do this (how to you test for "no crash of the JVM"). Well, you don't, because the crash isn't there. Here is his commit :

# test debugSupportingFiles
          ./bin/generate-samples.sh ./bin/configs/python.yaml -- --global-property debugSupportingFiles

A word of conclusion

Most of what I've learnt about OpenAPI happened through this bug. The folks on Slack are super helpful and they've been gentle giving directions while expecting me to do the work.

I write a lot of open code, but I don't work much in other people's libraries and that felt super nice. the bug should be closed in the coming weeks I expect.

Thanks everyone, and happy new year already ! 🎉

Don't hesitate to reach out if you have any questions! I'm mostly available on Mastodon and Linkedin those days 😊, though you can still find me on Twitter...