Skip to content

feat(core): add SOCKS5 proxy support for package management and remote providers#16900

Open
Ibochkarev wants to merge 3 commits intomodxcms:3.xfrom
Ibochkarev:feat/socks5-proxy-support
Open

feat(core): add SOCKS5 proxy support for package management and remote providers#16900
Ibochkarev wants to merge 3 commits intomodxcms:3.xfrom
Ibochkarev:feat/socks5-proxy-support

Conversation

@Ibochkarev
Copy link
Collaborator

What does it do?

  • Adds a new system setting proxy_type (HTTP, SOCKS4, SOCKS5, SOCKS5_HOSTNAME) so package manager, remote providers, and dashboard feed can use SOCKS proxies.
  • Introduces modX::getProxyType() and modX::getProxyUrl(); proxy URL is built with rawurlencode() for username/password to support special characters.
  • Uses the proxy URL in: Guzzle HTTP client options (buildHttpClientOptions), modTransportPackage::transferPackage() (cURL), and Dashboard Feed widget (SimplePie cURL options).
  • For HTTP proxy with auth, CURLOPT_PROXYAUTH and CURLOPT_PROXYUSERPWD are set only when proxy_type is HTTP (SOCKS auth is via URL).
  • Adds upgrade script 3.2.2-add-proxy-type-setting.php (and mysql/3.2.2-pl.php) so existing installations get the new setting.
  • Replaces deprecated curl_close($ch) with unset($ch) in modTransportPackage (PHP 8.4+).
  • New setting is installed via _build/data/transport.core.system_settings.php for fresh installs; English lexicon entries for proxy_type in core/lexicon/en/setting.inc.php.

Why is it needed?

Package downloads and dashboard feeds could not use SOCKS5 (or SOCKS4) proxies; only HTTP proxy was effectively supported. This change allows users behind SOCKS proxies to use package management and remote providers.

How to test

  1. Set System Settings (e.g. Proxy area): proxy_host, proxy_port, optionally proxy_username/proxy_password, and proxy_type to SOCKS5 (or HTTP, SOCKS4, SOCKS5_HOSTNAME).
  2. Install or update a package from a provider (Package Management) and confirm download succeeds via the proxy.
  3. Open the dashboard and confirm the MODX News/Security feed loads when a proxy is configured.
  4. Run upgrade from a pre-3.2.2 install and confirm the new proxy_type setting appears with default HTTP.

Related issue(s)/PR(s)

Resolves #16815

@Ibochkarev Ibochkarev marked this pull request as ready for review February 25, 2026 16:01
@codecov
Copy link

codecov bot commented Feb 25, 2026

Codecov Report

❌ Patch coverage is 16.66667% with 40 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.62%. Comparing base (af7a969) to head (d323f5e).
⚠️ Report is 10 commits behind head on 3.x.

Files with missing lines Patch % Lines
core/src/Revolution/modX.php 25.80% 23 Missing ⚠️
...e/src/Revolution/Transport/modTransportPackage.php 0.00% 13 Missing ⚠️
...lution/Processors/System/Dashboard/Widget/Feed.php 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                3.x   #16900      +/-   ##
============================================
- Coverage     21.64%   21.62%   -0.03%     
- Complexity    10764    10787      +23     
============================================
  Files           566      566              
  Lines         33005    33158     +153     
============================================
+ Hits           7144     7170      +26     
- Misses        25861    25988     +127     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…e providers

- Add proxy_type system setting (HTTP, SOCKS4, SOCKS5, SOCKS5_HOSTNAME)
- Add modX::getProxyType() and getProxyUrl() with credential encoding
- Use getProxyUrl() in Guzzle options, modTransportPackage, and Dashboard Feed
- Add upgrade script 3.2.1-add-proxy-type-setting.php and mysql/3.2.1-pl.php
- Replace deprecated curl_close() with unset() in modTransportPackage
- Fix phpcs: space after function keyword, line length in upgrade script

Resolves modxcms#16815
@Ibochkarev Ibochkarev force-pushed the feat/socks5-proxy-support branch from d323f5e to e052ed1 Compare February 25, 2026 16:03
@biz87
Copy link

biz87 commented Feb 25, 2026

Code Review

Summary

Adds SOCKS5/SOCKS4 proxy support for Package Manager, remote providers, and dashboard feed. Introduces proxy_type system setting and modX::getProxyType() / modX::getProxyUrl() methods, centralizing proxy logic that was previously duplicated across three files. 8 files, +189/-43. Includes unit tests.

Issues

modTransportPackage.php:477-483 — HTTP proxy credentials are encoded twice.
getProxyUrl() already includes rawurlencode(user):rawurlencode(pass)@host in the URL. But for HTTP proxies, CURLOPT_PROXYUSERPWD is set with another rawurlencode() call on the same values. This double-encoding will break authentication for passwords containing special characters (e.g. p@ss becomes p%2540ss instead of p%40ss).

Feed.php:148 and modTransportPackage.php:472CURLOPT_HTTPPROXYTUNNEL is set unconditionally for all proxy types.
HTTP CONNECT tunneling is appropriate for HTTP proxies with HTTPS targets but is unnecessary for SOCKS proxies and may cause issues with some implementations. Consider setting it only when proxy_type === 'HTTP'.

Suggestions

  • proxy_type is created as a textfield. For a fixed set of values (HTTP, SOCKS4, SOCKS5, SOCKS5_HOSTNAME), a dropdown/combobox xtype would be more user-friendly and prevent invalid input.

  • The test for socks5hHTTP (fallback) suggests socks5h is not accepted as input. Since it's a standard curl notation for SOCKS5 with remote DNS, it might be worth mapping it to SOCKS5_HOSTNAME instead of falling back to HTTP.

Assessment

Good refactoring — proxy logic is centralized, tests are solid. The double-encoding issue for HTTP proxy credentials is a real bug that will affect users with special characters in passwords. PHPStan clean.

Verdict

Request changes — double rawurlencode for HTTP proxy credentials will break authentication.

…configuration

- Update modX::getProxyType() to map 'SOCKS5H' to 'SOCKS5_HOSTNAME'.
- Adjust proxy configuration in modTransportPackage and Dashboard Feed to only set CURLOPT_HTTPPROXYTUNNEL for HTTP proxy types.
- Modify unit tests to reflect the change in expected proxy type behavior.
@Ibochkarev Ibochkarev added the feature Request about implementing a brand new function or possibility. label Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Request about implementing a brand new function or possibility.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add SOCKS5 proxy support for MODX3 package management and remote providers

2 participants