Refactor nginx path resolution with improved regex and fallback (#1414)

* 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.

* enhance: nginx path parsing and add tests #1412, #1414

---------

Co-authored-by: 0xJacky <[email protected]>

Author: enwikuna

internal/nginx/resolve_path.go

@@ -1,8 +1,8 @@
 package nginx
 
 import (
+	"os"
 	"path/filepath"
-	"regexp"
 	"runtime"
 	"strings"
 
@@ -34,16 +34,62 @@ 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 != "" {
 		return nginxPrefix
 	}
 
 	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()
@@ -53,21 +99,29 @@ func GetPrefix() string {
 		return nginxPrefix
 	}
 
-	nginxPrefix = resolvePath(match[1])
+	nginxPrefix = resolvePath(prefix)
 	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 ""
+		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)
 		}
-		confPath = match[1]
 	} else {
 		confPath = settings.NginxSettings.ConfigDir
 	}
@@ -81,35 +135,59 @@ func GetConfPath(dir ...string) (confPath string) {
 	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 "<confDir>/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 = 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 = ""
+			}
 		}
-		path = match[1]
 	} 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 = 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 ""
+			}
 		}
-		path = match[1]
 	} else {
 		path = settings.NginxSettings.PIDPath
 	}
@@ -128,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
@@ -159,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
@@ -189,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)
 		}
 	}
 

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)
+	}
+}