Skip to content

proxy: fix panic on invalid protocol enum#498

Open
fakhriaunur wants to merge 1 commit into
AdguardTeam:masterfrom
fakhriaunur:fix/h2-panic-error
Open

proxy: fix panic on invalid protocol enum#498
fakhriaunur wants to merge 1 commit into
AdguardTeam:masterfrom
fakhriaunur:fix/h2-panic-error

Conversation

@fakhriaunur
Copy link
Copy Markdown

Summary

Fix panic on invalid protocol enum in Addrs() and Addr() methods by returning an error instead.

When an invalid protocol enum is passed to Addrs() or Addr(), the methods currently panic with a string message. This change replaces the panic with a proper error return using errors.ErrBadEnumValue, following the existing error handling pattern in the codebase.

Changes

  • Change Addrs() method signature to return (addrs []net.Addr, err error)
  • Change Addr() method signature to return (addr net.Addr, err error)
  • Replace panic() in default case with return nil, fmt.Errorf("proto: %w: %q", errors.ErrBadEnumValue, proto)
  • Update all 19 call sites across 10 test files to handle the error return
  • Add table-driven tests for invalid enum error handling:
    • TestProxy_Addr_InvalidProto
    • TestProxy_Addrs_InvalidProto

Root Cause

The TODO comment in proxy/proxy.go:506 explicitly suggests using ErrBadEnumValue:

// TODO(e.burkov):  Use [errors.ErrBadEnumValue].
panic("proto must be 'tcp', 'tls', 'https', 'quic', 'dnscrypt' or 'udp'")

Testing

# Run tests
go test -v -run "TestProxy_Addr_InvalidProto|TestProxy_Addrs_InvalidProto" ./proxy/
# Output: PASS (2 test cases)

# Run race detection
go test -race ./proxy/
# Output: PASS (no races detected)

# Run lint
go vet ./proxy/
# Output: PASS

Code Change Statistics

  • Files modified: 11 (1 core file + 10 test files)
  • Lines changed: +67/-19
  • Test coverage: Addr() 90%, Addrs() improved

Technical Details

The fix follows the established error handling pattern in golibs/errors:

return nil, fmt.Errorf("proto: %w: %q", errors.ErrBadEnumValue, proto)

This pattern is already used in proxy/config.go:304 for upstream mode validation.

Issues

Related to H2 in contrib-cat.md

Checklist

  • Tests pass (go test ./proxy/)
  • No race conditions (go test -race ./proxy/)
  • Lint passes (go vet ./proxy/)
  • Coverage ≥80% for modified functions
  • Code change follows existing error patterns
  • Addresses existing TODO comment

Note: This is a minimal fix that replaces panic with proper error handling. The method signatures have been updated to return errors, and all existing call sites have been updated to handle the new return values.

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.

1 participant