Pass proxy from environment to jvm args

[aufi]

It was reported that proxy setup from environment variables is not correctly recognized by jdtls.

The jdtls startup python script now transforms those settings to JVM parameters.

Fixes: #172

Created mostly with claude code assistant.

Summary by CodeRabbit

Release Notes

• New Features

  • HTTP and HTTPS proxy configuration now supported via environment variables
  • Automatic handling of proxy credentials, ports, and non-proxy host exclusions
  • Proper environment variable precedence and URL parsing

• Tests

  • Added comprehensive test coverage for proxy configuration scenarios and edge cases

[coderabbitai]

Walkthrough

Added a new get_proxy_jvm_args() function that reads HTTP/HTTPS/NO_PROXY environment variables and transforms them into JVM proxy arguments. The function parses URLs, handles credentials, and converts NO_PROXY to Java-compatible format. Integrated into JVM startup sequence with comprehensive test coverage.

Changes

Cohort / File(s)	Summary
Production: Proxy argument builder jdtls-bin-override/jdtls.py	Added get_proxy_jvm_args() function that parses HTTP_PROXY, HTTPS_PROXY, NO_PROXY environment variables and constructs JVM proxy arguments. Imported urlparse from urllib.parse. Integrated proxy args into JVM startup before launching JDT LS jar.
Test suite: Proxy configuration tests jdtls-bin-override/test_jdtls.py	Added comprehensive test suite for get_proxy_jvm_args() covering: empty variables, HTTP/HTTPS proxies with/without ports and credentials, NO_PROXY single/multiple hosts, combined proxies, precedence rules, URL handling without schemes, invalid URLs, and edge cases. Tests use pytest and mock.patch.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

• URL parsing logic: Verify correct handling of urlparse() with credentials, ports, and missing schemes
• NO_PROXY transformation: Confirm regex/string transformation correctly converts comma-separated hosts to Java's http.nonProxyHosts format
• Environment variable precedence: Validate uppercase variable preference over lowercase variants
• Error handling: Check that invalid URLs (missing hostname) produce no arguments rather than errors
• Test coverage verification: Ensure test cases adequately validate all code paths in the new function

Poem

🐰 Hop, hop, proxies now flow,
Through JVM args, let packets go!
NO_PROXY transforms, credentials aligned,
Maven repos on isolated networks? Designed! 🌐

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: implementing proxy environment variable to JVM arguments transformation for jdtls startup.
Linked Issues check	✅ Passed	The PR successfully implements all coding requirements from issue #172: parsing proxy environment variables and converting them to JVM parameters for jdtls to recognize proxy configurations.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the stated objective: adding proxy environment variable handling to the jdtls startup script with comprehensive test coverage.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

jdtls-bin-override/jdtls.py (1)

113-117: Consider handling additional NO_PROXY separators.

The current implementation converts comma-separated NO_PROXY values to pipe-separated format for Java. However, some environments use spaces, semicolons, or mixed separators in NO_PROXY. Consider normalizing these as well for broader compatibility.

Example enhancement:

 	# NO_PROXY / Non-proxy hosts
 	no_proxy = os.environ.get('NO_PROXY') or os.environ.get('no_proxy')
 	if no_proxy:
-		# Convert comma-separated list to pipe-separated for Java
-		no_proxy_hosts = no_proxy.replace(',', '|')
+		# Convert various separators to pipe-separated for Java
+		import re
+		no_proxy_hosts = re.sub(r'[,\s;]+', '|', no_proxy.strip())
 		jvm_args.append(f'-Dhttp.nonProxyHosts={no_proxy_hosts}')

jdtls-bin-override/test_jdtls.py (1)

10-180: Excellent test coverage with comprehensive scenarios.

The test suite covers all major cases including:

• Basic proxy configurations (HTTP/HTTPS with/without ports)
• Case sensitivity and precedence rules
• Authentication credentials
• NO_PROXY handling
• Edge cases (missing schemes, invalid URLs, empty values)

Consider adding test for special characters in credentials.

Proxy passwords may contain special characters that require URL encoding (e.g., @, :, /, ?, %). Consider adding a test to verify these are handled correctly by urlparse.

Example test to add:

def test_proxy_with_special_characters_in_password(self):
    """Test proxy with special characters in password."""
    with patch.dict(os.environ, {'HTTP_PROXY': 'http://user:p@ss%2Fw0rd@proxy.example.com:8080'}, clear=True):
        result = get_proxy_jvm_args()
        assert '-Dhttp.proxyHost=proxy.example.com' in result
        assert '-Dhttp.proxyPort=8080' in result
        assert '-Dhttp.proxyUser=user' in result
        # urlparse should decode the password
        assert '-Dhttp.proxyPassword=p@ss/w0rd' in result

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 997d5d3 and 05f8c76.

📒 Files selected for processing (2)

• jdtls-bin-override/jdtls.py (3 hunks)
• jdtls-bin-override/test_jdtls.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

jdtls-bin-override/test_jdtls.py (1)

jdtls-bin-override/jdtls.py (1)

• get_proxy_jvm_args (73-119)

🔇 Additional comments (4)

jdtls-bin-override/jdtls.py (3)

21-21: LGTM: Import added for proxy URL parsing.

The urlparse import is correctly placed and necessary for parsing proxy environment variables.

---

155-155: LGTM: Proxy arguments correctly integrated into JVM startup.

The proxy arguments are properly positioned between base JVM arguments and user-provided arguments, allowing users to override proxy settings if needed via --jvm-arg.

---

89-92: Add documentation warning about proxy credential visibility in process listings.

The concern is valid: proxy passwords in jdtls.py lines 89-92 and 108-110 are passed as JVM arguments (-Dhttp.proxyPassword/-Dhttps.proxyPassword), making them visible in process listings. This follows Java's standard proxy configuration approach but represents a security trade-off in less trusted environments.

Add a comment above the get_proxy_jvm_args() function documenting this limitation, e.g.: "Note: proxy credentials are passed as JVM arguments and will be visible in process listings. This is Java's standard proxy configuration mechanism but consider alternatives in sensitive environments."

jdtls-bin-override/test_jdtls.py (1)

1-8: LGTM: Test suite structure is well-organized.

The test suite uses appropriate imports and follows pytest conventions with clear test class organization and descriptive docstrings.

[aufi]

Supporting also proxy without scheme prefix (http:// or https:// based on variable it is read from).

[shawn-hurley]

@aufi I think this is good change, I do wonder if we could use: -Djava.net.useSystemProxies=true and use the env vars on the operating system?

Furthermore, If this is run in a container in kantra, then we need to verify kantra sets the proxy env vars on the container or the provider config.

Also I am pretty sure, that you should also change: https://github.com/konveyor/analyzer-lsp/blob/main/external-providers/java-external-provider/pkg/java_external_provider/provider.go#L386 as we no longer use the the python script.

[aufi]

Thanks for input @shawn-hurley! actually it looks args were passed correctly from analyzer's external java provider, just in wrong order, so JVM ignored it. Should be solved with konveyor/analyzer-lsp#978