Skip to content

fix(net): guard HeaderToMap against header keys with empty value slices#4855

Open
anxkhn wants to merge 1 commit into
dragonflyoss:mainfrom
anxkhn:fix/guard-headertomap-against-header-keys
Open

fix(net): guard HeaderToMap against header keys with empty value slices#4855
anxkhn wants to merge 1 commit into
dragonflyoss:mainfrom
anxkhn:fix/guard-headertomap-against-header-keys

Conversation

@anxkhn

@anxkhn anxkhn commented Jul 1, 2026

Copy link
Copy Markdown

Description

HeaderToMap converted an http.Header to a map[string]string by indexing
v[0] for every key. http.Header is map[string][]string, so a key with an
empty value slice (e.g. h["X-Empty"] = []string{}) makes v[0] panic with
index out of range [0] with length 0. Such empty entries are preserved by
http.Header.Clone(). This guards the index with if len(v) > 0, mirroring the
defensive style of the sibling MapToHeader, and skips valueless keys. A
table-driven test case for a key with an empty value slice is added.

Related Issue

Fixes #4854

Motivation and Context

HeaderToMap is an exported helper in a reusable package. Indexing v[0]
unconditionally panics on a valid http.Header whose key has an empty value
slice (legal per the map[string][]string shape and kept by Clone()). Dropping
those keys instead of crashing keeps the helper safe on any valid input.

The sole production caller currently builds headers via
MapToHeader(map[string]string) (internal/job/image.go), so every key has at
least one value and cannot trigger the panic today. This is a defensive
correctness fix, not a known live crash.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation Update (if none of the other choices apply)

Checklist

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

HeaderToMap indexed v[0] for every header value, which panics with
"index out of range [0] with length 0" when an http.Header key has an
empty value slice (e.g. h["X-Empty"] = []string{}). Such entries are
preserved by http.Header.Clone(). Skip keys with no values, mirroring the
defensive style of the sibling MapToHeader. Add a table-driven test case
covering a key with an empty value slice.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 28.04%. Comparing base (0b3cca4) to head (a4fd03e).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4855   +/-   ##
=======================================
  Coverage   28.04%   28.04%           
=======================================
  Files         232      232           
  Lines       23148    23149    +1     
=======================================
+ Hits         6491     6492    +1     
  Misses      16218    16218           
  Partials      439      439           
Flag Coverage Δ
unittests 28.04% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pkg/net/http/http.go 82.14% <100.00%> (+0.66%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pmady

pmady commented Jul 1, 2026

Copy link
Copy Markdown

yeah this would panic if a header key somehow ended up with an empty slice. one-liner fix + test, clean.

one thought should the empty-slice key just be skipped entirely (like you did), or should it be set to "" in the map? currently it gets dropped which changes the key set. probably fine for how HeaderToMap is used but worth a think

@anxkhn

anxkhn commented Jul 6, 2026

Copy link
Copy Markdown
Author

Good question. I went with dropping the key rather than mapping it to "" because map[string]string can't represent "key present with no value" the way map[string][]string can, so there's no lossless option here. Dropping keeps HeaderToMap consistent with MapToHeader/http.Header.Set, which never produce a key with an empty value in the first place, so a round trip through both functions stays stable. Setting "" would fabricate a value that was never actually sent, which felt like the worse of the two imperfect options. Happy to switch if you'd prefer the "" behavior instead.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the exported helper HeaderToMap in pkg/net/http to avoid a panic when an http.Header contains a key whose value slice is empty (a valid state for map[string][]string). It aligns HeaderToMap with defensive handling expectations for reusable utilities and adds a regression test for the edge case described in issue #4854.

Changes:

  • Prevent HeaderToMap from indexing v[0] when a header key has an empty value slice (skip such keys).
  • Add a table-driven test case covering headers that include an empty value slice entry.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/net/http/http.go Adds a len(v) > 0 guard to prevent HeaderToMap panics on empty value slices.
pkg/net/http/http_test.go Adds a regression test ensuring keys with empty value slices are skipped.

@yxxhero yxxhero enabled auto-merge (squash) July 6, 2026 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HeaderToMap panics on a header key with an empty value slice

6 participants