Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data Race when handling simultaneously requests #775

Closed
wrong-user opened this issue Mar 9, 2023 · 3 comments · Fixed by #811
Closed

Data Race when handling simultaneously requests #775

wrong-user opened this issue Mar 9, 2023 · 3 comments · Fixed by #811

Comments

@wrong-user
Copy link

I am facing an Data Race when I am sending simultaneously request against my service that is using

func ValidateRequest(ctx context.Context, input *RequestValidationInput) (err error) {
to validate the incoming requests.

The OpenApi specification is loaded by the service. Validation of the request against the spec is handle in a middleware before it is handle by the service itself, or reject if it does not adhere to the spec.

The race is occurring because of a write at

if schema.compiledPattern, err = regexp.Compile(schema.Pattern); err != nil {
and a read at
if cp := schema.compiledPattern; cp != nil && !cp.MatchString(value) {

Out of curiosity I added a mutex in the schema struct and applied it in as below.

// Schema.go:1599
schema.PatternMu.Lock()
if schema.Pattern != "" && schema.compiledPattern == nil && !settings.patternValidationDisabled {
  var err error
  if err = schema.compilePattern(); err != nil {
    if !settings.multiError {
      return err
    }
    me = append(me, err)
  }
}
schema.PatternMu.Unlock()

I am running it with v.0.114.0 at commit ecb06bc, but the issue is also present in older versions.

@fenollp
Copy link
Collaborator

fenollp commented Mar 10, 2023

Hello please show us: open a PR with a test that reproduces your issue. Thanks.

@wrong-user
Copy link
Author

Sure, I will do that.

@fenollp fenollp linked a pull request Mar 18, 2023 that will close this issue
@fenollp
Copy link
Collaborator

fenollp commented Jun 20, 2023

Hey so I have a fix, two actually!

Short term: doc.Validate(ctx) before building your router. You should do that anyway. It pre compiles all patterns so in effect fixes this issue.

Longer term: I'll propose a patch that relies on a package-global sync.Map.

But do validate your docs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants