rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 6 Wiki Activity

Address review feedback on PR #113
PR Ready Gate / clear-labels (pull_request) Successful in 2s
Details
CI / test (pull_request) Successful in 22s
Details
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 34s
Details
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m33s
Details
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m47s
Details

Browse Source 
- Fix AllowInsecureHTTP doc comment: say '_test.go file in the same
  package' instead of 'export_test.go' (MINOR #1)
- Remove dead u.Fragment = "" from redactURL: HTTP requests never
  carry fragments over the wire per RFC 7230 §5.1 (MINOR #2)
- Use 127.0.0.1:1 in scheme-rejection tests to make intent clearer
  that no network call should occur (NIT #3)
This commit is contained in:
claw
2026-05-13 13:04:23 -07:00
parent 6b7f3f6924
commit ab2a6c8aef
2 changed files with 5 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
github/client.go
+1 -2
View File
@@ -155,7 +155,7 @@ type clientConfig struct {
// environment variable. Without the env var set, the option is ignored // environment variable. Without the env var set, the option is ignored
// and a warning is logged. // and a warning is logged.
// //
// For tests, use AllowInsecureHTTPForTest (defined in export_test.go) which bypasses the env gate. // For tests, use AllowInsecureHTTPForTest (defined in a _test.go file in the same package) which bypasses the env gate.
func AllowInsecureHTTP() ClientOption { func AllowInsecureHTTP() ClientOption {
return func(cfg *clientConfig) { return func(cfg *clientConfig) {
cfg.allowInsecureHTTP = true cfg.allowInsecureHTTP = true
@@ -272,7 +272,6 @@ func redactURL(rawURL string) string {
if u.RawQuery != "" { if u.RawQuery != "" {
u.RawQuery = "<redacted>" u.RawQuery = "<redacted>"
} }
u.Fragment = ""
return u.String() return u.String()
} }
github/client_test.go
+4 -4
View File
@@ -547,8 +547,8 @@ func TestNoInsecureOption_RejectsHTTP(t *testing.T) {
func TestNoInsecureOption_RejectsUppercaseHTTP(t *testing.T) { func TestNoInsecureOption_RejectsUppercaseHTTP(t *testing.T) {
// Verify case-insensitive scheme check (RFC 3986). // Verify case-insensitive scheme check (RFC 3986).
c := NewClient("tok", "HTTP://example.com") c := NewClient("tok", "HTTP://127.0.0.1:1")
_, err := c.doGet(context.Background(), "HTTP://example.com/test") _, err := c.doGet(context.Background(), "HTTP://127.0.0.1:1/test")
if err == nil { if err == nil {
t.Fatal("expected error for uppercase HTTP scheme") t.Fatal("expected error for uppercase HTTP scheme")
} }
@@ -559,8 +559,8 @@ func TestNoInsecureOption_RejectsUppercaseHTTP(t *testing.T) {
func TestNoInsecureOption_RejectsMixedCaseHTTP(t *testing.T) { func TestNoInsecureOption_RejectsMixedCaseHTTP(t *testing.T) {
// Verify mixed case like "Http://" is also rejected. // Verify mixed case like "Http://" is also rejected.
c := NewClient("tok", "Http://example.com") c := NewClient("tok", "Http://127.0.0.1:1")
_, err := c.doGet(context.Background(), "Http://example.com/test") _, err := c.doGet(context.Background(), "Http://127.0.0.1:1/test")
if err == nil { if err == nil {
t.Fatal("expected error for mixed-case HTTP scheme") t.Fatal("expected error for mixed-case HTTP scheme")
} }