segmentation fault when attempting to generate from spec that contains `allOf` groups

[rogerluan]

Hey 👋

I've recently spent some time fighting with SwagGen because it would enter a recursion when attempting to parse a given json spec.

After much debugging, I narrowed down the problem to the presence of allOf groups in my spec.

I read a few issues and PRs in this repo, did some shallow code review in key parts of the code base to try to understand what problem the presence allOf was causing, that was triggering the recursion. Some of the issues/PRs: #221, #286, #267, #269, #217, #278. I couldn't figure out why it's crashing (entering a recursion) on my specific spec file 😬 Specially because the sample specs present in this repo do feature allOf groups, so I don't know what specific configuration in my spec files is causing this particular problem 😞

I tested with v4.3.1, v4.4.0, and v4.6.0, all of which caused the same problem (segmentation fault - a recursion somewhere).

If it helps at all, here's the stack trace (click to expand):

Line 0	Line 1	Line 2	Line 3	Line 4

Workaround

As a workaround for my specific case, since all of the allOf occurrences had a single element in the array, I replaced extracted its $ref out of the allOf key, directly in the JSON file, before parsing it using SwagGen. In Ruby, here's the script that worked for me:

# Taken from https://stackoverflow.com/a/4174125/4075379
def regex_replace_file(filename, regexp, replacement)
  require 'tempfile'
  Tempfile.open(".#{File.basename(filename)}", File.dirname(filename)) do |tempfile|
    File.open(filename).each do |line|
      tempfile.puts(line.gsub(regexp, replacement))
    end
    tempfile.close
    FileUtils.mv(tempfile.path, filename)
  end
end

swagger_spec_path = "/path/to/your/spec.json"
regexp = /"allOf":\[\{("\$ref":"#\/components\/schemas\/[a-zA-Z]+")\}\]/m
regex_replace_file(swagger_spec_path, regexp, '\1')

# swagger_spec_path now contains the processed JSON file, ready to be consumed by SwagGen

This workaround is admittedly terrible, but in my specific use case it didn't break anything. Use with caution.

[rogerluan]

One side effect I noticed when doing this deliberate manual modification of the swagger json is that SwagGen started ignoring the "nullable": true specifiers for those $ref (that were previously allOf references). So that if I had a JSON that looked something like this:

"myProperty": {
    "$ref": "#/components/schemas/MyClass",
    "nullable": true
}

It would generate a nonnull property instead of a nullable one, like this:

public var myProperty: MyClass

Interestingly enough, for specific cases I was able to test the allOf reference (instead of regex-replacing it with the raw $ref) and it happened to not crash in this particular place I replaced. Then, the result was the expected one:

public var myProperty: MyClass?

While debugging SwagGen I realized that if I modified this part of the code:

SwagGen/Sources/Swagger/Schema/ObjectSchema.swift

Lines 44 to 52 in 2b3140b

	public extension Property {
	var nullable: Bool {
	if case let .reference(ref) = schema.type {
	return !required || ref.value.metadata.nullable
	} else {
	return !required || schema.metadata.nullable
	}
	}
	}

With this:

public extension Property {
    var nullable: Bool {
        return !required || schema.metadata.nullable
    }
}

Everything works as expected.

Any clue as to why there's this special case handling for when the schema type is a reference (which was committed here: b464a08) @yonaskolb @alephao? 🙏

[alephao]

@rogerluan Can you share your swagger file or a minimal example where this issue happen? Maybe I can help

BTW, the JSON you posted is not following the swagger specs, nullability should be defined in the object itself. I know, it doesn't make any sense, but that's the spec

"myProperty": {
    "$ref": "#/components/schemas/MyClass",
    "nullable": true
}

reference: https://swagger.io/docs/specification/using-ref/

$ref and Sibling Elements
Any sibling elements of a $ref are ignored. This is because $ref works by replacing itself and everything on its level with the definition it is pointing at. Consider this example:

[rogerluan]

Thanks for getting back to this issue @alephao ! 🙇

Can you share your swagger file or a minimal example where this issue happen?

I was afraid someone would ask for it 😅 because unfortunately I can't share my spec, and as I wasn't able to identify what's wrong, I couldn't recreate it 😬 I know this makes it significantly harder for us to come to a solution 😢 but if you have debugging steps you'd like me to take, I'm happy to try out solutions and help debugging in any way I can!

---

Thanks for that reference about the swagger spec on $ref and its siblings elements. I didn't know about that. Perhaps https://editor.swagger.io should validate for the presence of sibling elements and at least throw a warning 😬 I won't even bother opening the PR of my fork since that's just conceptually wrong now, but unfortunately I'll have to keep using the code I changed since it solves my specific problems with the parser 🙇

[alephao]

I understand, but I won't be very helpful without something that helps me debugging 😔. Perhaps try searching for a reference cycle in your swagger file, that's what the issue you described sounds like to me.

[rogerluan]

So, actually there are cyclic references in my swagger file, but that doesn't seem to be the problem when I'm not using allOf validator, so idk if that's really the issue here. I'll explain:

If MyClass has references (properties) to MyOtherClass and MyOtherClass has references to MyClass, should that be considered a cyclic reference? I thought that that was the reason of the recursion, but if I remove the allOf references, replacing them by regular $ref references (as I noted in my original post), everything works fine. So it's probably a bug somewhere in the allOf validator, no? 😬