Skip to content

fix(profile): Add nil check to profile validation#15325

Open
raykroeker wants to merge 7 commits into
mainfrom
raykroeker/profile-nil-pointer
Open

fix(profile): Add nil check to profile validation#15325
raykroeker wants to merge 7 commits into
mainfrom
raykroeker/profile-nil-pointer

Conversation

@raykroeker
Copy link
Copy Markdown
Contributor

@raykroeker raykroeker commented May 27, 2026

  • Check for nil: routes, response class, request match for any/all, response match for any/all
  • Add tests to cover each.
  • Add tests to cover duration parsing pass/fail.

Signed-off-by: Raymond Kroeker raymond@buoyant.io

 * Add nil check to spec routes in the service profile.
 * Add test with nil route yaml.
@raykroeker raykroeker requested a review from a team as a code owner May 27, 2026 20:19
Copy link
Copy Markdown
Member

@adleong adleong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! There are a few more places in this codepath that are vulnerable to nil pointer dereferences that we should clean up also:

Both ValidateRequestMatch and ValidateResponseMatch take pointers as input and call themselves recursively but don't guard against calling with a nil pointer.

Comment thread pkg/profiles/profiles.go
return fmt.Errorf("ServiceProfile %q has a route with an invalid condition: %w", serviceProfile.Name, err)
}
for _, rc := range route.ResponseClasses {
if rc.Condition == nil {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have the same issue here if rc is nil, this will panic.

 * Check for nil: response class, request match for any/all, response
   match for any/all
 * Add tests to cover each.
 * Update existing wording to state 'null' vs 'nil'
@raykroeker
Copy link
Copy Markdown
Contributor Author

@adleong All nil pointers should be covered now. Thanks for the review.

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.

2 participants