Proxy support for outgoing http request - #214
Conversation
| ); | ||
|
|
||
| if ($use_proxy) { | ||
| $http_settings['proxy'] = "tcp://" . $proxy_host . ":" . $proxy_port; |
There was a problem hiding this comment.
using tcp:// seems too restrictive here. Could you add a variable $proxy_protocol?
Also, please test if proxy_port is set before adding ":" . $proxy_port
There was a problem hiding this comment.
Thanks for your code review.
Adding isset() test to both variables is a very good practice 👍
PHP official documentation for stream-context-create leads to HTTP context options. It's not crystal clear to me if we can use another scheme.
So I tested it with a local squid :
- in
htdocs/geocode.php, when replacingtcp://byhttp://, it doesn't work and throw the following error
[Wed Feb 11 21:33:21.771370 2026] [php:notice] [pid 335:tid 335] [client X.X.X.X:36754] PHP Parse error: syntax error, unexpected token "and" in /var/www/htdocs/geocode.php on line 33, referer: http://X.X.X.X:8080/index.php?page=map
- while a simple curl test using
https_proxyenv var is OK :$ https_proxy="http://mysquidproxy.local:3128" curl https://google.fr -v -4
I therefore suggest keeping tcp:// hardcoded, as the library does not appear to support other protocol schemes.
I would be happy to modify it if you find a counter-example.
| if ($use_proxy) { | ||
| $http_settings['proxy'] = "tcp://" . $proxy_host . ":" . $proxy_port; | ||
| if ($proxy_use_ssl) { | ||
| $http_settings['request_fulluri'] = true; |
There was a problem hiding this comment.
This has nothing to do with ssl, right? We may enable this option, but I recommend renaming it into $proxy_request_fulluri
Anyway, for ssl proxy options, I suggest using the 'ssl' key of http_settings, and having an array $proxy_ssl_options in configuration for storing the complete ssl options. A commented example of such configuration could be set in config.inc.php.
There was a problem hiding this comment.
OK, sounds good to me. I will propose that
| if ($proxy_use_ssl) { | ||
| $http_settings['request_fulluri'] = true; | ||
| } | ||
| if ($proxy_auth) { |
There was a problem hiding this comment.
I think we should rename $proxy_auth into $proxy_auth_authorization_method, as there are multiple authentication schemes. The values could be: "none" (default) and "basic".
There was a problem hiding this comment.
Generally speaking, authorization are checked on the server side, after authentication.
In our use case, we are providing credentials for authentication on the proxy.
So i would propose $proxy_authentication_method
You are absolutely right, an HTTP proxy can provide multiple authentication methods : Basic, NTLM, SPNEGO, OAuth2, ... (ref squid doc).
From my experience :
- only few softwares like
curlor browsers are supporting other authentication methods than Basic, - HTTP proxies dedicated to servers or workloads almost always offer basic authentication.
So i would propose $proxy_authentication_method, and only support the none and basic options to start with..
| $proxy_auth = false; | ||
| $proxy_auth_user = "user"; | ||
| $proxy_auth_pass = "password"; | ||
|
|
There was a problem hiding this comment.
The parameters seems self-explanatory here, but we should describe them in the documentation as well.
There was a problem hiding this comment.
I propose an update on docs/mapmenu.rst, i hope it will fit what you expect
|
please note that i still need to test basic auth with squid, i'll inform you when it's validated. |
|
Tests with squid proxy + basic auth are now done, and it's working. note : tests with SSL Bump (TLS interception) could be done following this repository, but i didn't plan to cover it at this stage. |
2 participants
See description in ticket #213