Feature Request: ValidateHttpRequestSync

[commoddity]

Background:
We were using github.com/pb33f/libopenapi-validator in our production code and receiving occasional panics, which I believe match the error shown in #75 (now solved).

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x88 pc=0x2aad6c9]
goroutine 128159 [running]:
github.com/pb33f/libopenapi-validator/requests.ValidateRequestSchema(0xc0a612cb00, 0xc0a377fb80, {0xc08e17d7a0, 0x3, 0x8}, {0xc08e17d7b8, 0x2, 0xc000680000?})
    /go/pkg/mod/github.com/pb33f/[email protected]/requests/validate_request.go:89 +0x469
github.com/pb33f/libopenapi-validator/requests.(*requestBodyValidator).ValidateRequestBody(0xc04fa91a80, 0xc0a612cb00)
    /go/pkg/mod/github.com/pb33f/[email protected]/requests/validate_body.go:91 +0x4a6
github.com/pb33f/libopenapi-validator.(*validator).ValidateHttpRequest.func2(0x717285?, 0xc04bc7a420?)
    /go/pkg/mod/github.com/pb33f/[email protected]/validator.go:247 +0x30
created by github.com/pb33f/libopenapi-validator.(*validator).ValidateHttpRequest in goroutine 48754
    /go/pkg/mod/github.com/pb33f/[email protected]/validator.go:267 +0x346

These panics were causing a fatal crash in our production code, so we attempted to wrap the method call to ValidateHttpRequest in a recover() in order to ensure it could not result in crashing our application.

After digging into the codebase, we discovered the recover() did not work because ValidateHttpRequest spawns multiple goroutines in order to perform the request validation and thus panics emitted from these goroutines could not be recovered from in our code.

We solved the issue by forking the repo and creating a ValidateHttpRequestSync method that performs the validation synchronously, without any goroutines. This allows the method call to be safely wrapped in recover().

While I believe the particular panic we were seeing has been solved, for use in production we needed to have absolute certainty that a panic or nil-pointer deref emitted in this lib could not lead to a fatal crash of our application.

It's also worth noting that the time savings between ValidateHttpRequest and ValidateHttpRequestSync were negligible in my opinion (16ms vs 21ms, respectively, in my tests). ValidateHttpRequestSync from our fork is currently in use in our production codebase without any issue.

Request:

Add the ValidateHttpRequestSync method to the library so that the user of the library can determine if & how they wish to use goroutines for the request validation, as opposed to goroutines being spawned inside the library, which can lead to unrecoverable panics as shown above.

Example:

Here is the implementation of ValidateHttpRequestSync from our fork:
https://github.com/pokt-foundation/libopenapi-validator/blob/main/validator.go#L284

[daveshanley]

Why not contribute this back to the library? all PRs are welcome!

[commoddity]

Why not contribute this back to the library? all PRs are welcome!

I'm happy to! Just opened up a PR here that includes the method requested above:
#80

Let me know what you think when you have time to review. Thanks. :)

[daveshanley]

Added in v0.0.55 Thank you for your contribution!