fix(go): Basic Auth header not sent when username is empty#13563
fix(go): Basic Auth header not sent when username is empty#13563iamnamananand996 merged 4 commits intomainfrom
Conversation
…ader
Per RFC 7617, an empty user-id is valid in Basic Auth. The previous
condition required both username and password to be non-empty, which
silently dropped the Authorization header when username was empty
(e.g., WithBasicAuth("", "my-api-key")).
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward one-line logic fix (&& → ||) consistent with RFC 7617, with matching seed and changelog updates.
Extended reasoning...
Overview
This PR changes a single conditional operator in the Go SDK generator (sdk.go:429) from && to || for the Basic Auth header guard, so the Authorization header is emitted whenever either the username or password is non-empty. The same change is propagated to the test fixture (testdata/sdk/basic/fixtures/core/request_option.go) and three seed output files (any-auth, basic-auth, endpoint-security-auth). A versions.yml entry for v1.30.2 with type: fix is included.
Security risks
The behavioral change means WithBasicAuth("user", "") or WithBasicAuth("", "password") will now send an Authorization header where previously it would silently drop it. This is the correct behavior per RFC 7617 and aligns with how other HTTP clients handle Basic Auth. No new security surface is introduced.
Level of scrutiny
This is a low-risk, mechanical one-line operator change in a code generator. The intent is clear, the RFC justification is sound, and the seed outputs confirm the generator produces the expected output. Minimal scrutiny is needed.
Other factors
The inline bug comment about the pre-existing ": " vs ":" discrepancy in the test fixture is valid but was not introduced by this PR — the PR author explicitly documents this in the Reviewer Checklist. The versions.yml entry follows the repository conventions with the correct type: fix and irVersion: 61.
| header := r.cloneHeader() | ||
| if r.Username != "" && r.Password != "" { | ||
| if r.Username != "" || r.Password != "" { | ||
| header.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(r.Username+": "+r.Password))) |
There was a problem hiding this comment.
🟣 Pre-existing bug: The test fixture uses r.Username+": "+r.Password (space after colon) but the generator at sdk.go:430 produces r.Username+":"+r.Password (no space). This was supposed to be fixed in v1.16.5 ("Fix Basic auth header formatting to remove erroneous whitespace") but this fixture was missed. Since this PR already modifies this line, it should also fix ": " to ":" to match the generator output and all seed files.
Extended reasoning...
What the bug is
The test fixture at generators/go/internal/testdata/sdk/basic/fixtures/core/request_option.go line 52 constructs the Basic Auth credential string with a space after the colon separator:
header.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(r.Username+": "+r.Password)))while the generator at generators/go/internal/generator/sdk.go line 430 produces:
r.Username + ":" + r.Password(no space after the colon). All three seed outputs modified in this PR (any-auth, basic-auth, endpoint-security-auth) also use ":" without a space, consistent with the generator.
Step-by-step proof
- The generator template at sdk.go:430 emits:
r.Username + ":" + r.Password - The seed outputs (e.g.,
seed/go-sdk/basic-auth/core/request_option.goline 53) all contain:r.Username+":"+r.Password - The fixture at
testdata/sdk/basic/fixtures/core/request_option.goline 52 contains:r.Username+": "+r.Password(note the space after colon) - For credentials
user/pass, the fixture would producebase64("user: pass")=dXNlcjogcGFzcw==instead of the correctbase64("user:pass")=dXNlcjpwYXNz - RFC 7617 specifies the format as
user-id ":" passwordwith no whitespace around the colon
Why this was missed
Version 1.16.5 in versions.yml explicitly states: "Fix Basic auth header formatting to remove erroneous whitespace between username and password." This fix was applied to the generator and seed outputs but the test fixture in testdata/sdk/basic/ was overlooked.
Impact
The fixture produces incorrect Basic Auth headers with an extra space in the credentials, violating RFC 7617. If the cmdtest.TestFixtures framework ever runs a byte-for-byte comparison of generated output against this fixture, it would either fail (because the generator produces ":" but the fixture has ": ") or, if the fixture is used as compilable Go code directly, it would produce malformed auth headers at runtime.
Addressing the duplicate concern
One verifier noted this may be a duplicate of bug_002. This report covers bug_001 as assigned; if bug_002 describes the same issue, only one comment needs to be posted. The underlying issue remains valid regardless.
Fix
On line 52 of generators/go/internal/testdata/sdk/basic/fixtures/core/request_option.go, change r.Username+": "+r.Password to r.Username+":"+r.Password to match the generator output.
There was a problem hiding this comment.
Good catch — fixed in 5a9f505. Changed ": " to ":" in the test fixture to match the generator output, seed files, and RFC 7617.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Per user feedback, remove the conditional check entirely instead of changing && to ||. The Authorization header is now always set when Basic Auth is configured, matching RFC 7617 which considers even empty username:password as valid credentials.
… auth Removing the if guard entirely would overwrite Bearer/API key auth in multi-auth SDKs. The || approach correctly sets the Basic Auth header when either username or password is non-empty while preserving other auth mechanisms when Basic Auth is not configured.
The test fixture used ': ' (space after colon) instead of ':' in the Basic Auth credential string, inconsistent with the generator output and all seed files. This was missed when v1.16.5 fixed the same issue in the generator. Per RFC 7617 the format is user-id:password with no whitespace around the colon.
iamnamananand996
deleted the
devin/1773669098-fix-go-basic-auth-empty-username
branch
2 participants
Description
Fixes a bug where
WithBasicAuth("", "my-api-key")silently dropped theAuthorizationheader because the generatedToHeader()required both username and password to be non-empty (&&). Per RFC 7617, an empty user-id is a valid Basic Auth credential (e.g.,:password).Changes Made
sdk.go) from&&to||, so theAuthorizationheader is set when either field is non-emptytestdata/sdk/basic) and seed outputs (any-auth,basic-auth,endpoint-security-auth)": "→":"(extra space after colon) in the test fixturetestdata/sdk/basic/fixtures/core/request_option.go, aligning it with the generator output and all seed files per RFC 76171.30.2changelog entry ingenerators/go/sdk/versions.ymlTesting
go test ./internal/...)Human Review Checklist
||semantics cover the desired cases:WithBasicAuth("", "pass")✅ sends header;WithBasicAuth("user", "")✅ sends header;WithBasicAuth("", "")❌ does not send header (both fields empty). The reporter noted that supporting fully-empty credentials (":") would require more significant changes.Link to Devin session: https://app.devin.ai/sessions/860e57bb8f7a47978117231dfb6cf3a0
Requested by: @iamnamananand996