Skip to content

fix: support lowercase proxy environment variables as fallback#746

Open
EvanYao826 wants to merge 2 commits into
langgenius:mainfrom
EvanYao826:main
Open

fix: support lowercase proxy environment variables as fallback#746
EvanYao826 wants to merge 2 commits into
langgenius:mainfrom
EvanYao826:main

Conversation

@EvanYao826
Copy link
Copy Markdown

@EvanYao826 EvanYao826 commented May 23, 2026

Summary

The plugin_daemon config only reads 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 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 existing setDefaultString helper function for proxy environment variable fallback to maintain code consistency.

Checklist

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
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working go Pull requests that update go code plugin-daemon labels May 23, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread internal/types/app/default.go Outdated
Comment on lines +12 to +20
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")
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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"))

@EvanYao826
Copy link
Copy Markdown
Author

Thank you for the review! I will update the PR to use the existing setDefaultString helper function for consistency.

@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working go Pull requests that update go code plugin-daemon size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant