[beskvladimir-create]

Description

getHeaders failed whenever an optional header param was omitted. ParseParams appends a ParamValue for every declared parameter, so the omitted param reaches getHeaders as a nil entry in paramsMap; the code only checked key presence and then failed the nil.(string) assertion, returning "error populating request headers" for every such call.

This change skips nil header values, matching how getURL already treats optional query params. A unit test (http_headers_test.go) builds paramsMap through ParseParams, as Invoke does, and covers both the omitted and the provided case.

Note: open PR #3472 touches the adjacent else line (%t → %T); this PR does not change that line, so the two do not overlap in intent.

Security: header values are still taken only from declared parameters; no new input path is added.

Testing: I could not run go test locally (my environment cannot download the Go 1.25.8 toolchain); the change is gofmt-clean and CI will run the tests.

PR Checklist

• Make sure you reviewed CONTRIBUTING.md
• Make sure to open an issue as a bug/issue before writing your code
• Ensure you have manually reviewed the entire diff before requesting a review
• Ensure the tests and linter pass (left to CI, see Testing above)
• Code coverage does not decrease (unit test added)
• Appropriate docs were updated (no docs change needed)
• Make sure to add ! if this involve a breaking change (not breaking)

🛠️ Fixes #4334

[gemini-code-assist]

Code Review

This pull request modifies the getHeaders function in internal/tools/http/http.go to omit optional header parameters that are present in the parameters map with a nil value. Additionally, a new test file internal/tools/http/http_headers_test.go is added to verify this behavior with unit tests. There are no review comments, so I have no feedback to provide.

[Yuan325]

Thank you for your contributions @beskvladimir-create Will approve once feedback is addressed.

[Yuan325]

Thank you. I've added a test case to make sure value with empty string is also filtered.

[Yuan325]

/gcbrun

[Yuan325]

/gcbrun

[Yuan325]

Hi @beskvladimir-create The unit tests is failing for another test case, can you please take a look at that. And also, should we be filtering empty string values too? Seems like the test case that I added was failing. Thank you again for your contributions!

[beskvladimir-create]

Thanks @Yuan325! Both failures came from the same change, and I've pushed two commits to address them:

• TestGetHeadersCaseInsensitiveOverride (added in fix(http): preserve case-insensitive header precedence #4417) had a case "nil parameter retains existing error behavior" that asserted the old error for a nil value. A nil value is exactly how an omitted optional header param reaches getHeaders, so I updated that case to expect the static header to be kept, and added a case that a non-string value still returns an error.
• Yes, filtering empty strings makes sense to me: an empty value carries no header and would otherwise override a source/tool header with the same name. getHeaders now skips "" as well, so your empty string header provided case should pass, and I added an empty-string case to the getHeaders table too. Note this also applies to required header params (an empty value is omitted rather than sent as an empty header); happy to limit it to optional params if you prefer.

I couldn't run the Go test suite locally (no Go 1.27 toolchain in my environment); I checked the new getHeaders logic against these cases with a standalone copy, so CI is the real check here.