From 8b565a423f1db9d87478c17dfa623ce9dfe8afa6 Mon Sep 17 00:00:00 2001 From: enwikuna <166232905+enwikuna@users.noreply.github.com> Date: Wed, 29 Oct 2025 12:52:34 +0100 Subject: [PATCH 1/2] Refactor nginx path resolution with improved regex and fallback Updated regex patterns for extracting nginx configuration paths and added fallback mechanisms for determining paths on different operating systems. Improved error handling and logging for better debugging. --- internal/nginx/resolve_path.go | 143 +++++++++++++++++++++------------ 1 file changed, 92 insertions(+), 51 deletions(-) diff --git a/internal/nginx/resolve_path.go b/internal/nginx/resolve_path.go index f99be16eb..5816a95d8 100644 --- a/internal/nginx/resolve_path.go +++ b/internal/nginx/resolve_path.go @@ -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" + } + + logger.Debug("nginx.GetConfPath fallback used", "base", confPath) + } + } 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 } -// GetConfEntryPath returns the path of the nginx configuration file +// GetConfEntryPath returns the absolute path to the main nginx.conf. +// It prefers the value from `nginx -V --conf-path=...`. +// If that can't be parsed, it falls back to "/nginx.conf". func GetConfEntryPath() (path string) { - if settings.NginxSettings.ConfigPath == "" { - out := getNginxV() - r, _ := regexp.Compile("--conf-path=(.*.conf)") - match := r.FindStringSubmatch(out) - if len(match) < 1 { - logger.Error("nginx.GetConfEntryPath len(match) < 1") - return "" - } - path = match[1] - } else { - path = settings.NginxSettings.ConfigPath - } - - return resolvePath(path) + if settings.NginxSettings.ConfigPath == "" { + out := getNginxV() + r, _ := regexp.Compile(`--conf-path=([^\s]+)`) + match := r.FindStringSubmatch(out) + + if len(match) > 1 { + path = match[1] + } else { + baseDir := GetConfPath() + + if baseDir != "" { + path = filepath.Join(baseDir, "nginx.conf") + } else { + logger.Error("nginx.GetConfEntryPath: cannot determine nginx.conf path") + path = "" + } + } + } else { + path = settings.NginxSettings.ConfigPath + } + + return resolvePath(path) } -// GetPIDPath returns the path of the nginx PID file +// GetPIDPath returns the nginx master process PID file path. +// We try to read it from `nginx -V --pid-path=...`. +// If that fails (which often happens in container images), we probe common +// locations like /run/nginx.pid and /var/run/nginx.pid instead of just failing. func GetPIDPath() (path string) { - if settings.NginxSettings.PIDPath == "" { - out := getNginxV() - r, _ := regexp.Compile("--pid-path=(.*.pid)") - match := r.FindStringSubmatch(out) - if len(match) < 1 { - logger.Error("pid path not found in nginx -V output") - return "" - } - path = match[1] - } else { - path = settings.NginxSettings.PIDPath - } - - return resolvePath(path) + if settings.NginxSettings.PIDPath == "" { + out := getNginxV() + r, _ := regexp.Compile(`--pid-path=([^\s]+)`) + match := r.FindStringSubmatch(out) + + if len(match) > 1 { + path = match[1] + } else { + candidates := []string{ + "/var/run/nginx.pid", + "/run/nginx.pid", + } + + for _, c := range candidates { + if _, err := os.Stat(c); err == nil { + logger.Debug("GetPIDPath fallback hit", "path", c) + path = c + break + } + } + + if path == "" { + logger.Error("GetPIDPath: could not determine PID path") + return "" + } + } + } else { + path = settings.NginxSettings.PIDPath + } + + return resolvePath(path) } // GetSbinPath returns the path of the nginx executable From f5df9be6496fa38ea8baaf35aa6726d8af68f5ea Mon Sep 17 00:00:00 2001 From: 0xJacky Date: Tue, 25 Nov 2025 13:30:31 +0800 Subject: [PATCH 2/2] enhance: nginx path parsing and add tests #1412, #1414 --- api/cluster/node.go | 9 +- internal/nginx/resolve_path.go | 233 +++++++++++++++------------- internal/nginx/resolve_path_test.go | 101 ++++++++++++ 3 files changed, 231 insertions(+), 112 deletions(-) create mode 100644 internal/nginx/resolve_path_test.go diff --git a/api/cluster/node.go b/api/cluster/node.go index 3c5be796b..7f88308a1 100644 --- a/api/cluster/node.go +++ b/api/cluster/node.go @@ -45,14 +45,7 @@ func GetNodeList(c *gin.Context) { return analytic.GetNode(m) }) - data, ok := core.ListAllData() - if !ok { - return - } - - c.JSON(http.StatusOK, model.DataList{ - Data: data, - }) + core.List() } func AddNode(c *gin.Context) { diff --git a/internal/nginx/resolve_path.go b/internal/nginx/resolve_path.go index 5816a95d8..969860a60 100644 --- a/internal/nginx/resolve_path.go +++ b/internal/nginx/resolve_path.go @@ -3,7 +3,6 @@ package nginx import ( "os" "path/filepath" - "regexp" "runtime" "strings" @@ -35,6 +34,53 @@ func resolvePath(path string) string { return path } +func extractConfigureArg(out, flag string) string { + if out == "" || flag == "" { + return "" + } + + if !strings.HasPrefix(flag, "--") { + flag = "--" + flag + } + + needle := flag + "=" + idx := strings.Index(out, needle) + if idx == -1 { + return "" + } + + start := idx + len(needle) + if start >= len(out) { + return "" + } + + value := out[start:] + value = strings.TrimLeft(value, " \t") + if value == "" { + return "" + } + + if value[0] == '"' || value[0] == '\'' { + quoteChar := value[0] + rest := value[1:] + closingIdx := strings.IndexByte(rest, quoteChar) + if closingIdx == -1 { + return strings.TrimSpace(rest) + } + return strings.TrimSpace(rest[:closingIdx]) + } + + cut := len(value) + if idx := strings.Index(value, " --"); idx != -1 && idx < cut { + cut = idx + } + if idx := strings.IndexAny(value, "\r\n"); idx != -1 && idx < cut { + cut = idx + } + + return strings.TrimSpace(value[:cut]) +} + // GetPrefix returns the prefix of the nginx executable func GetPrefix() string { if nginxPrefix != "" { @@ -42,9 +88,8 @@ func GetPrefix() string { } out := getNginxV() - r, _ := regexp.Compile(`--prefix=(\S+)`) - match := r.FindStringSubmatch(out) - if len(match) < 1 { + prefix := extractConfigureArg(out, "--prefix") + if prefix == "" { logger.Debug("nginx.GetPrefix len(match) < 1") if runtime.GOOS == "windows" { nginxPrefix = GetNginxExeDir() @@ -54,7 +99,7 @@ func GetPrefix() string { return nginxPrefix } - nginxPrefix = resolvePath(match[1]) + nginxPrefix = resolvePath(prefix) return nginxPrefix } @@ -62,62 +107,57 @@ func GetPrefix() string { // 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=([^\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" - } - - logger.Debug("nginx.GetConfPath fallback used", "base", confPath) - } - } 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() + fullConf := extractConfigureArg(out, "--conf-path") + + if fullConf != "" { + confPath = filepath.Dir(fullConf) + } else { + if runtime.GOOS == "windows" { + confPath = GetPrefix() + } else { + confPath = "/etc/nginx" + } + + logger.Debug("nginx.GetConfPath fallback used", "base", confPath) + } + } 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 } // GetConfEntryPath returns the absolute path to the main nginx.conf. // It prefers the value from `nginx -V --conf-path=...`. // If that can't be parsed, it falls back to "/nginx.conf". func GetConfEntryPath() (path string) { - if settings.NginxSettings.ConfigPath == "" { - out := getNginxV() - r, _ := regexp.Compile(`--conf-path=([^\s]+)`) - match := r.FindStringSubmatch(out) - - if len(match) > 1 { - path = match[1] - } else { - baseDir := GetConfPath() - - if baseDir != "" { - path = filepath.Join(baseDir, "nginx.conf") - } else { - logger.Error("nginx.GetConfEntryPath: cannot determine nginx.conf path") - path = "" - } - } - } else { - path = settings.NginxSettings.ConfigPath - } - - return resolvePath(path) + if settings.NginxSettings.ConfigPath == "" { + out := getNginxV() + path = extractConfigureArg(out, "--conf-path") + + if path == "" { + baseDir := GetConfPath() + + if baseDir != "" { + path = filepath.Join(baseDir, "nginx.conf") + } else { + logger.Error("nginx.GetConfEntryPath: cannot determine nginx.conf path") + path = "" + } + } + } else { + path = settings.NginxSettings.ConfigPath + } + + return resolvePath(path) } // GetPIDPath returns the nginx master process PID file path. @@ -125,37 +165,34 @@ func GetConfEntryPath() (path string) { // If that fails (which often happens in container images), we probe common // locations like /run/nginx.pid and /var/run/nginx.pid instead of just failing. func GetPIDPath() (path string) { - if settings.NginxSettings.PIDPath == "" { - out := getNginxV() - r, _ := regexp.Compile(`--pid-path=([^\s]+)`) - match := r.FindStringSubmatch(out) - - if len(match) > 1 { - path = match[1] - } else { - candidates := []string{ - "/var/run/nginx.pid", - "/run/nginx.pid", - } - - for _, c := range candidates { - if _, err := os.Stat(c); err == nil { - logger.Debug("GetPIDPath fallback hit", "path", c) - path = c - break - } - } - - if path == "" { - logger.Error("GetPIDPath: could not determine PID path") - return "" - } - } - } else { - path = settings.NginxSettings.PIDPath - } - - return resolvePath(path) + if settings.NginxSettings.PIDPath == "" { + out := getNginxV() + path = extractConfigureArg(out, "--pid-path") + + if path == "" { + candidates := []string{ + "/var/run/nginx.pid", + "/run/nginx.pid", + } + + for _, c := range candidates { + if _, err := os.Stat(c); err == nil { + logger.Debug("GetPIDPath fallback hit", "path", c) + path = c + break + } + } + + if path == "" { + logger.Error("GetPIDPath: could not determine PID path") + return "" + } + } + } else { + path = settings.NginxSettings.PIDPath + } + + return resolvePath(path) } // GetSbinPath returns the path of the nginx executable @@ -169,10 +206,8 @@ func GetAccessLogPath() (path string) { if path == "" { out := getNginxV() - r, _ := regexp.Compile(`--http-log-path=(\S+)`) - match := r.FindStringSubmatch(out) - if len(match) > 1 { - path = match[1] + path = extractConfigureArg(out, "--http-log-path") + if path != "" { resolvedPath := resolvePath(path) // Check if the matched path exists but is not a regular file @@ -200,10 +235,8 @@ func GetErrorLogPath() string { if path == "" { out := getNginxV() - r, _ := regexp.Compile(`--error-log-path=(\S+)`) - match := r.FindStringSubmatch(out) - if len(match) > 1 { - path = match[1] + path = extractConfigureArg(out, "--error-log-path") + if path != "" { resolvedPath := resolvePath(path) // Check if the matched path exists but is not a regular file @@ -230,16 +263,8 @@ func GetModulesPath() string { // First try to get from nginx -V output out := getNginxV() if out != "" { - // Look for --modules-path in the output - if strings.Contains(out, "--modules-path=") { - parts := strings.Split(out, "--modules-path=") - if len(parts) > 1 { - // Extract the path - path := strings.Split(parts[1], " ")[0] - // Remove quotes if present - path = strings.Trim(path, "\"") - return resolvePath(path) - } + if path := extractConfigureArg(out, "--modules-path"); path != "" { + return resolvePath(path) } } diff --git a/internal/nginx/resolve_path_test.go b/internal/nginx/resolve_path_test.go new file mode 100644 index 000000000..87ccbe801 --- /dev/null +++ b/internal/nginx/resolve_path_test.go @@ -0,0 +1,101 @@ +package nginx + +import ( + "fmt" + "path/filepath" + "testing" + + "github.com/0xJacky/Nginx-UI/settings" +) + +func TestExtractConfigureArg(t *testing.T) { + t.Parallel() + + output := ` +nginx version: nginx/1.25.2 +configure arguments: --prefix="/Program Files/Nginx" --conf-path='/Program Files/Nginx/conf/nginx.conf' --pid-path=/var/run/nginx.pid +` + + tests := []struct { + name string + flag string + want string + }{ + { + name: "double quoted conf path", + flag: "--conf-path", + want: "/Program Files/Nginx/conf/nginx.conf", + }, + { + name: "single quoted conf path alias", + flag: "conf-path", + want: "/Program Files/Nginx/conf/nginx.conf", + }, + { + name: "unquoted pid path", + flag: "pid-path", + want: "/var/run/nginx.pid", + }, + { + name: "missing flag", + flag: "--http-log-path", + want: "", + }, + { + name: "prefix parsing", + flag: "prefix", + want: "/Program Files/Nginx", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + if got := extractConfigureArg(output, tt.flag); got != tt.want { + t.Fatalf("extractConfigureArg(%q) = %q, want %q", tt.flag, got, tt.want) + } + }) + } +} + +func TestGetConfAndPidPathsHandleSpaces(t *testing.T) { + originalConfigDir := settings.NginxSettings.ConfigDir + originalConfigPath := settings.NginxSettings.ConfigPath + originalPIDPath := settings.NginxSettings.PIDPath + originalNginxVOutput := nginxVOutput + + t.Cleanup(func() { + settings.NginxSettings.ConfigDir = originalConfigDir + settings.NginxSettings.ConfigPath = originalConfigPath + settings.NginxSettings.PIDPath = originalPIDPath + nginxVOutput = originalNginxVOutput + }) + + settings.NginxSettings.ConfigDir = "" + settings.NginxSettings.ConfigPath = "" + settings.NginxSettings.PIDPath = "" + + sampleConf := "/Program Files/nginx/conf/nginx.conf" + samplePID := "/Program Files/nginx/logs/nginx.pid" + + nginxVOutput = fmt.Sprintf(` +nginx version: nginx/1.25.2 +configure arguments: --conf-path="%s" --pid-path="%s" +`, sampleConf, samplePID) + + confDir := GetConfPath() + expectedConfDir := filepath.Dir(sampleConf) + if confDir != expectedConfDir { + t.Fatalf("GetConfPath() = %q, want %q", confDir, expectedConfDir) + } + + confEntry := GetConfEntryPath() + if confEntry != sampleConf { + t.Fatalf("GetConfEntryPath() = %q, want %q", confEntry, sampleConf) + } + + pidPath := GetPIDPath() + if pidPath != samplePID { + t.Fatalf("GetPIDPath() = %q, want %q", pidPath, samplePID) + } +}