Skip to content

Fix invalid nil interface check on NewRequest - #464

Merged
optik-aper merged 4 commits into
vultr:masterfrom
optik-aper:fix-nil-interface-json-body
Jul 13, 2026
Merged

Fix invalid nil interface check on NewRequest#464
optik-aper merged 4 commits into
vultr:masterfrom
optik-aper:fix-nil-interface-json-body

Conversation

@optik-aper

@optik-aper optik-aper commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Description

The nil check when building the request body from the interface in NewRequest will always pass, even when the interface is nil.

Interfaces must be opened in order to check if nil. The issue meant that the JSON marshaler would always output 'null' when a nil interface was passed in to NewRequest.

Related Issues

Checklist:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you linted your code locally prior to submission?
  • Have you successfully ran tests with your changes locally?

Interfaces must be opened in order to check if nil. The issue meant that
the JSON marshaler would always output 'null' when a nil interface was
passed in to NewRequest.
@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Panic on nil interface

reflect.ValueOf(body).IsNil() panics when body is a nil interface (i.e., the zero Value). The original code correctly handled this case with body != nil. Any caller passing a literal nil for the body parameter will now trigger a runtime panic instead of skipping the encoding step. The fix should retain the nil-interface guard, e.g., if body != nil && !reflect.ValueOf(body).IsNil().

buf := &bytes.Buffer{}
if !reflect.ValueOf(body).IsNil() {
Panic on non-pointer body

reflect.Value.IsNil() only works for chan, func, interface, map, pointer, and slice kinds; it panics for all other types. If a caller passes a non-pointer value such as a struct, string, or int as body, the code will panic. The nil-check should verify the value's Kind is one of the nilable kinds before calling IsNil().

buf := &bytes.Buffer{}
if !reflect.ValueOf(body).IsNil() {

@optik-aper optik-aper self-assigned this Jul 13, 2026
@optik-aper optik-aper added the bug Something isn't working label Jul 13, 2026
@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix nil check panic by preserving nil guard

The reflection-based nil check reflect.ValueOf(body).IsNil() will panic when body is
nil because calling IsNil() on an invalid reflect.Value (resulting from
reflect.ValueOf(nil)) causes a panic. The original body != nil guard is still
necessary to prevent this panic before attempting the reflection check.

govultr.go [171]

-if !reflect.ValueOf(body).IsNil() {
+if body != nil && !reflect.ValueOf(body).IsNil() {
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that reflect.ValueOf(body).IsNil() will panic when body is nil (an invalid reflect.Value causes panic on IsNil()). The suggested fix body != nil && !reflect.ValueOf(body).IsNil() is valid and prevents this crash while preserving the PR's intent to handle nil pointers via reflection. This is a critical bug that could cause runtime panics in production.

High

@optik-aper optik-aper changed the title Fix invalid nil interface check Fix invalid nil interface check on NewRequest Jul 13, 2026
@optik-aper
optik-aper merged commit 480cbc6 into vultr:master Jul 13, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant