fix(soax): fix config copy bug, certverifier bug, doc typo, add HTTP CONNECT test and SOAX integration tests#588
fix(soax): fix config copy bug, certverifier bug, doc typo, add HTTP CONNECT test and SOAX integration tests#588
Conversation
- Fix NewSession() to copy the config struct rather than the pointer, so session defaults no longer mutate the original ProxySessionConfig - Fix doc comment on NewWebProxyStreamDialer (was "NewStreamDialer") - Fix license header typo in proxy_test.go - Add opts parameter to NewWebProxyStreamDialer for transport configurability (e.g. TLS options), and add a test that validates credentials are correctly sent via Proxy-Authorization header Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
fortuna
force-pushed
the
fix/soax-proxy-fixes
branch
from
a8a6d86 to
5421a0b
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes several issues in the x/soax package: a config copy bug in NewSession() where the pointer was being copied instead of the struct (allowing session defaults to mutate the original ProxySessionConfig), a doc comment typo on NewWebProxyStreamDialer, a license header typo in the test file, and adds a variadic opts parameter to NewWebProxyStreamDialer for transport configurability along with a new HTTP CONNECT test.
Changes:
- Fix
NewSession()to perform a value copy of the config struct instead of a pointer copy, preventing mutation of the originalProxySessionConfig - Fix doc comment on
NewWebProxyStreamDialerand license header typo inproxy_test.go - Add variadic
opts ...httpconnect.TransportOptionparameter toNewWebProxyStreamDialerand add a test that validates credentials viaProxy-Authorizationheader
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| x/soax/proxy.go | Fix config struct copy (pointer → value), fix doc comment, add opts parameter to NewWebProxyStreamDialer |
| x/soax/proxy_test.go | Fix license header typo, add new imports, add TestSession_NewWebProxyStreamDialer test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…egration tests - Remove ConnType from Client struct; pass it directly to GetRegions and GetCities, which are the only methods that need it - Add api_integration_test.go with live API tests gated on SOAX_API_KEY and SOAX_PACKAGE_KEY environment variables - Document in README how to run integration tests and which package type each test requires Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gration tests - Fix nil pointer panic in toStdConfig when CertVerifier is not set, defaulting to StandardCertVerifier. Applied to both transport/tls and x/httpconnect. - Add live proxy integration tests for SOCKS5 and HTTP CONNECT via SOAX, gated on SOAX_PACKAGE_ID and SOAX_PACKAGE_KEY environment variables. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
fortuna
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When I tried to use the SOAX library I created before, I ran into multiple issues.
This revamps it addressing a few problems.
ProxySessionConfig.NewSession()— the session configwas copying a pointer, so mutations to session-specific fields (session ID, duration,
idle TTL) would alias back to the original config.
toStdConfig(bothtransport/tlsandx/httpconnect) — when no TLS options were set,CertVerifierwas nil, causing anil pointer dereference during TLS handshake. Now defaults to
StandardCertVerifier.NewWebProxyStreamDialer(wasNewStreamDialer).TransportOptionforwarding toNewWebProxyStreamDialer, enabling callersto customize TLS (or use plain HTTP for testing).
ConnTypefromClientstruct toGetRegions/GetCitiesparameters —avoids mutable state on the client and makes the package type explicit at the call site.
NewWebProxyStreamDialerusing a local test server.api_integration_test.go) and bothproxy protocols (
proxy_integration_test.go), gated on environment variables.