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

Fix: Prevent panic when validating struct with nil pointer to map field #260

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

kaptinlin
Copy link
Contributor

This pull request addresses a issue within the gookit/validate library where attempting to validate a struct containing a nil pointer to a map field results in a runtime panic.

Problem:
The library encounters a panic with the error message: reflect: call of reflect.Value.MapKeys on zero Value, when it attempts to validate a struct field that is a nil pointer to a map. This occurs due to the invocation of MapKeys() without prior validation of the reflect.Value being both non-nil and valid.

Solution:
A safeguard has been implemented to check that the reflect.Value is not nil and is valid before any call to MapKeys(). This modification ensures graceful handling of nil pointer fields, treating them as empty maps, which is the anticipated behavior for such cases in the validation process.

Example Reproduction and Fix Verification:

The issue and the effectiveness of the proposed fix can be demonstrated with the following example:

package main

import (
    "github.com/gookit/validate"
    "github.com/stretchr/testify/assert"
)

type TagRequest struct {
    Metadata *map[string]interface{} `validate:"required"`
}

func main() {
    data := &TagRequest{}
    v := validate.Struct(data)

    ok := v.Validate() // Prior to the fix, this call would panic
    assert.False(t, ok) // The assertion is expected to be false since Metadata is required but nil
}

With the implementation of the proposed fix, the Validate method now appropriately interprets the nil pointer to the map as an empty map. Consequently, the validation process proceeds without a panic, and the assertion evaluates as intended, reflecting the non-fulfillment of the required rule for the Metadata field due to its nil value.

This enhancement not only prevents undesirable application crashes but also aligns the library's behavior with the conventional handling of nil pointers in Go, thereby improving its reliability and usability.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 8107516187

Details

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.004%) to 96.68%

Totals Coverage Status
Change from base Build 7894771328: 0.004%
Covered Lines: 2912
Relevant Lines: 3012

💛 - Coveralls

@inhere inhere self-assigned this Mar 1, 2024
@inhere inhere merged commit 1822f1e into gookit:master Mar 1, 2024
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants