Skip to content

write/service: handle a nil URL from url.Parse without panicking#427

Open
c-tonneslan wants to merge 1 commit into
influxdata:masterfrom
c-tonneslan:fix-write-nil-url-panic
Open

write/service: handle a nil URL from url.Parse without panicking#427
c-tonneslan wants to merge 1 commit into
influxdata:masterfrom
c-tonneslan:fix-write-nil-url-panic

Conversation

@c-tonneslan
Copy link
Copy Markdown

Fixes #422. NewClient accepts any string for serverURL and only stores it. A malformed value like a bare host:port (192.168.1.104:8086) makes url.Parse return (nil, err). internal/write.NewService discards that error and immediately calls u.Parse("write") on the nil URL, panicking on first WriteAPIBlocking use.

Stack trace from the repro in the issue:

net/url.(*URL).ResolveReference(0x0, ...)
net/url.(*URL).Parse(0x0, "write")
.../internal/write.NewService(...) service.go:77
.../api.NewWriteAPIBlocking(...)
.../v2.(*clientImpl).WriteAPIBlocking(...)

Fall back to building the write URL by string concatenation when url.Parse fails. The bad address still surfaces later when the HTTP request is dispatched, just as a regular network/HTTP error instead of a panic.

Added a regression test that constructs NewService with the broken URL from the issue.

Fixes #422

NewClient accepts any string for serverURL and only stores it; a
malformed value like a bare host:port ("192.168.1.104:8086") makes
url.Parse return nil and an error. NewService discarded that error
and immediately called u.Parse("write") on the nil URL, taking the
whole program down with a nil pointer dereference.

Fall back to building the write URL by string concatenation when
url.Parse fails. The bad address still surfaces later when the HTTP
request is dispatched, just as a regular network/HTTP error instead
of a panic.

Fixes influxdata#422

Signed-off-by: Charlie Tonneslan <cst0520@gmail.com>
@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.99%. Comparing base (67f504e) to head (eb81762).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #427   +/-   ##
=======================================
  Coverage   92.99%   92.99%           
=======================================
  Files          25       25           
  Lines        2326     2326           
=======================================
  Hits         2163     2163           
  Misses        123      123           
  Partials       40       40           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Invalid server URL format (IP without scheme) results in panic while calling WriteAPIBlocking

2 participants