fix: support lowercase proxy environment variables as fallback#746
fix: support lowercase proxy environment variables as fallback#746EvanYao826 wants to merge 2 commits into
Conversation
The plugin_daemon config only read uppercase HTTP_PROXY, HTTPS_PROXY, and NO_PROXY via envconfig tags. When only lowercase http_proxy, https_proxy, and no_proxy are set (common in many environments), the proxy settings were silently ignored, causing plugin installation failures behind HTTP proxies. Add fallback in SetDefault() to check lowercase env vars when uppercase are not set, matching the behavior of other Dify containers (api, web). Fixes #18752
There was a problem hiding this comment.
Code Review
This pull request updates the SetDefault function in internal/types/app/default.go to include fallbacks for proxy environment variables (http_proxy, https_proxy, and no_proxy) when their uppercase counterparts are not set. The review feedback suggests utilizing the existing setDefaultString helper function to implement these fallbacks more concisely and maintain consistency with the rest of the function.
| if config.HttpProxy == "" { | ||
| config.HttpProxy = os.Getenv("http_proxy") | ||
| } | ||
| if config.HttpsProxy == "" { | ||
| config.HttpsProxy = os.Getenv("https_proxy") | ||
| } | ||
| if config.NoProxy == "" { | ||
| config.NoProxy = os.Getenv("no_proxy") | ||
| } |
There was a problem hiding this comment.
For better maintainability and consistency with the rest of the SetDefault function, use the existing setDefaultString helper. This reduces boilerplate while maintaining the same logic.
| if config.HttpProxy == "" { | |
| config.HttpProxy = os.Getenv("http_proxy") | |
| } | |
| if config.HttpsProxy == "" { | |
| config.HttpsProxy = os.Getenv("https_proxy") | |
| } | |
| if config.NoProxy == "" { | |
| config.NoProxy = os.Getenv("no_proxy") | |
| } | |
| setDefaultString(&config.HttpProxy, os.Getenv("http_proxy")) | |
| setDefaultString(&config.HttpsProxy, os.Getenv("https_proxy")) | |
| setDefaultString(&config.NoProxy, os.Getenv("no_proxy")) |
|
Thank you for the review! I will update the PR to use the existing setDefaultString helper function for consistency. |
Summary
The plugin_daemon config only reads uppercase
HTTP_PROXY,HTTPS_PROXY, andNO_PROXYvia envconfig tags. When only lowercasehttp_proxy,https_proxy, andno_proxyare set (common in many environments), the proxy settings are silently ignored, causing plugin installation failures behind HTTP proxies.Add fallback in
SetDefault()to check lowercase env vars when uppercase are not set, matching the behavior of other Dify containers (api, web).Fix
In
internal/types/app/default.go, use the existingsetDefaultStringhelper function for proxy environment variable fallback to maintain code consistency.Checklist