Support standard HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables#2631
Support standard HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables#2631kavix wants to merge 1 commit into
Conversation
…riables for HTTP client
📝 WalkthroughWalkthroughThis PR adds support for standard HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables to the Ballerina HTTP client. The implementation extends proxy configuration logic with environment-aware resolution, NO_PROXY bypass matching, and proper proxy URL parsing with error handling. ChangesHTTP Proxy Environment Variable Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
native/src/test/java/io/ballerina/stdlib/http/api/ProxyEnvVarTest.java (1)
55-84: ⚡ Quick winAdd a negative-path assertion for invalid proxy URLs.
Line 55 introduces parse tests, but Line 84 ends without covering the documented error-handling branch (
Invalid proxy URL). Add at least one malformed input assertion to lock down exception behavior and prevent regressions.Proposed test addition
`@Test` public void testParseProxyUrl() { @@ ProxyServerConfiguration config5 = HttpUtil.parseProxyUrl("https://proxy.example.com"); Assert.assertEquals(config5.getProxyHost(), "proxy.example.com"); Assert.assertEquals(config5.getProxyPort(), 443); + + Assert.expectThrows(BallerinaConnectorException.class, + () -> HttpUtil.parseProxyUrl("http://:invalid")); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@native/src/test/java/io/ballerina/stdlib/http/api/ProxyEnvVarTest.java` around lines 55 - 84, Add a negative-path assertion for HttpUtil.parseProxyUrl by verifying it rejects malformed proxy URLs (e.g., "http://:8080" or "not a url") and raises the documented Invalid proxy URL error; update the ProxyEnvVarTest (either extend testParseProxyUrl or add a new test like testParseProxyUrlInvalid) to assert that calling HttpUtil.parseProxyUrl with an invalid string throws the expected exception (e.g., IllegalArgumentException) so the invalid-URL branch is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@native/src/main/java/io/ballerina/stdlib/http/api/HttpUtil.java`:
- Around line 1417-1419: The catch block in HttpUtil.java that throws new
BallerinaConnectorException("Invalid proxy URL: " + proxyUrl, e) can leak
embedded credentials in proxyUrl; update the code to sanitize the proxyUrl
before including it in the exception message (e.g., remove or mask URI
user-info) and use the sanitized string in the thrown
BallerinaConnectorException; locate the proxy handling in HttpUtil (the catch
for MalformedURLException | UnknownHostException) and replace the direct
proxyUrl usage with a sanitizedProxyUrl produced by parsing the URL/URI and
stripping userInfo (or masking it) so sensitive credentials are not logged or
returned.
---
Nitpick comments:
In `@native/src/test/java/io/ballerina/stdlib/http/api/ProxyEnvVarTest.java`:
- Around line 55-84: Add a negative-path assertion for HttpUtil.parseProxyUrl by
verifying it rejects malformed proxy URLs (e.g., "http://:8080" or "not a url")
and raises the documented Invalid proxy URL error; update the ProxyEnvVarTest
(either extend testParseProxyUrl or add a new test like
testParseProxyUrlInvalid) to assert that calling HttpUtil.parseProxyUrl with an
invalid string throws the expected exception (e.g., IllegalArgumentException) so
the invalid-URL branch is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd3debb4-3d00-449e-af2c-b45c38f4e80c
📒 Files selected for processing (3)
native/src/main/java/io/ballerina/stdlib/http/api/HttpUtil.javanative/src/main/java/io/ballerina/stdlib/http/api/client/endpoint/CreateSimpleHttpClient.javanative/src/test/java/io/ballerina/stdlib/http/api/ProxyEnvVarTest.java



This PR adds support for the industry-standard HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables in the Ballerina HTTP client. Resolves ballerina-platform/ballerina-lang#44389.
Summary
This pull request adds support for standard HTTP proxy environment variables (
HTTP_PROXY,HTTPS_PROXY, andNO_PROXY) to the Ballerina HTTP client. The implementation enables automatic proxy detection and configuration from environment variables, eliminating the need for proxy settings to be embedded in application code. This enhances deployment flexibility, particularly in environments that rely on system or container-level proxy configuration.Changes
HttpUtil.java
populateSenderConfigurationsmethod to accept an additionaltargetHostparameter, enabling host-aware proxy resolutionHTTP_PROXY,HTTPS_PROXY) for proxy configurationCreateSimpleHttpClient.java
populateSenderConfigurationsto pass the target host extracted from the connection URL, enabling the proxy configuration logic to make host-specific decisionsProxyEnvVarTest.java