-
-
Notifications
You must be signed in to change notification settings - Fork 723
Refactor nginx path resolution with improved regex and fallback #1414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||
| package nginx | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||||||
| "os" | ||||||||||||||||||||||||||||||||||||||
| "path/filepath" | ||||||||||||||||||||||||||||||||||||||
| "regexp" | ||||||||||||||||||||||||||||||||||||||
| "runtime" | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -57,64 +58,104 @@ func GetPrefix() string { | |||||||||||||||||||||||||||||||||||||
| return nginxPrefix | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // GetConfPath returns the path of the nginx configuration file | ||||||||||||||||||||||||||||||||||||||
| // GetConfPath returns the nginx configuration directory (e.g. "/etc/nginx"). | ||||||||||||||||||||||||||||||||||||||
| // It tries to derive it from `nginx -V --conf-path=...`. | ||||||||||||||||||||||||||||||||||||||
| // If parsing fails, it falls back to a reasonable default instead of returning "". | ||||||||||||||||||||||||||||||||||||||
| func GetConfPath(dir ...string) (confPath string) { | ||||||||||||||||||||||||||||||||||||||
| if settings.NginxSettings.ConfigDir == "" { | ||||||||||||||||||||||||||||||||||||||
| out := getNginxV() | ||||||||||||||||||||||||||||||||||||||
| r, _ := regexp.Compile("--conf-path=(.*)/(.*.conf)") | ||||||||||||||||||||||||||||||||||||||
| match := r.FindStringSubmatch(out) | ||||||||||||||||||||||||||||||||||||||
| if len(match) < 1 { | ||||||||||||||||||||||||||||||||||||||
| logger.Error("nginx.GetConfPath len(match) < 1") | ||||||||||||||||||||||||||||||||||||||
| return "" | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| confPath = match[1] | ||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||
| confPath = settings.NginxSettings.ConfigDir | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| confPath = resolvePath(confPath) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| joined := filepath.Clean(filepath.Join(confPath, filepath.Join(dir...))) | ||||||||||||||||||||||||||||||||||||||
| if !helper.IsUnderDirectory(joined, confPath) { | ||||||||||||||||||||||||||||||||||||||
| return confPath | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| return joined | ||||||||||||||||||||||||||||||||||||||
| if settings.NginxSettings.ConfigDir == "" { | ||||||||||||||||||||||||||||||||||||||
| out := getNginxV() | ||||||||||||||||||||||||||||||||||||||
| r, _ := regexp.Compile(`--conf-path=([^\s]+)`) | ||||||||||||||||||||||||||||||||||||||
| match := r.FindStringSubmatch(out) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if len(match) > 1 { | ||||||||||||||||||||||||||||||||||||||
| fullConf := match[1] | ||||||||||||||||||||||||||||||||||||||
| confPath = filepath.Dir(fullConf) | ||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||
| if runtime.GOOS == "windows" { | ||||||||||||||||||||||||||||||||||||||
| confPath = GetPrefix() | ||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||
| confPath = "/etc/nginx" | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixed indentation detected: line 108 uses tabs while the surrounding lines use spaces. This should be standardized to match the project's indentation style.
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential infinite recursion: GetConfEntryPath calls GetConfPath() as a fallback (line 107), which could lead to circular dependency issues if GetConfPath ever needs to call GetConfEntryPath. While this may not happen in the current code, consider passing a flag or using a different approach to avoid this pattern.
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded candidate paths for PID files don't include Windows-specific locations. Consider adding Windows-specific candidates (e.g., relative to GetPrefix() or GetNginxExeDir()) or conditionally defining candidates based on runtime.GOOS to ensure cross-platform compatibility.
| candidates := []string{ | |
| "/var/run/nginx.pid", | |
| "/run/nginx.pid", | |
| var candidates []string | |
| if runtime.GOOS == "windows" { | |
| exeDir := GetNginxExeDir() | |
| prefix := GetPrefix() | |
| candidates = []string{ | |
| filepath.Join(exeDir, "nginx.pid"), | |
| filepath.Join(exeDir, "logs", "nginx.pid"), | |
| filepath.Join(prefix, "nginx.pid"), | |
| filepath.Join(prefix, "logs", "nginx.pid"), | |
| } | |
| } else { | |
| candidates = []string{ | |
| "/var/run/nginx.pid", | |
| "/run/nginx.pid", | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixed indentation detected: tabs and spaces are inconsistent. Line 69 uses tabs while the surrounding lines (65-68, 70-81) use spaces for indentation. This should be standardized to match the project's indentation style (appears to be tabs based on other parts of the file).