From 18fc8ce0781466e3d2cfc31feaa5c3a49ef83fbc Mon Sep 17 00:00:00 2001 From: maciekpaprocki Date: Wed, 1 Feb 2023 14:36:40 +0000 Subject: [PATCH 1/5] Added support for index.php in nested directories --- local/php/envs.go | 36 +++++--- local/php/envs_test.go | 90 ++++++++++++++++++- .../testdata/public/subdirectory/index.php | 0 .../subdirectory/subdirectory/index.php | 0 4 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 local/php/testdata/public/subdirectory/index.php create mode 100644 local/php/testdata/public/subdirectory/subdirectory/index.php diff --git a/local/php/envs.go b/local/php/envs.go index a16b4e76..d0bf19b5 100644 --- a/local/php/envs.go +++ b/local/php/envs.go @@ -29,22 +29,38 @@ import ( "github.com/symfony-cli/symfony-cli/envs" ) -func (p *Server) generateEnv(req *http.Request) map[string]string { - scriptName := p.passthru - https := "" - if req.TLS != nil { - https = "On" - } - - pathInfo := req.URL.Path +func (p *Server) resolveScriptName(pathInfo string) (string, string) { if pos := strings.Index(strings.ToLower(pathInfo), ".php"); pos != -1 { file := pathInfo[:pos+4] if _, err := os.Stat(filepath.Join(p.documentRoot, file)); err == nil { - scriptName = file - pathInfo = pathInfo[pos+4:] + return file, pathInfo[pos+4:] + } + } + + paths := strings.Split(strings.Trim(pathInfo, "/"), "/") + for n := len(paths); n > 0; n-- { + pathPart := paths[n-1] + if pathPart == "" { + continue + } + + file := filepath.Join(append(paths[:n], p.passthru)...) + if _, err := os.Stat(filepath.Join(p.documentRoot, file)); err == nil { + return "/" + file, pathInfo[strings.LastIndex(pathInfo, pathPart)+len(pathPart):] } } + return p.passthru, pathInfo +} + +func (p *Server) generateEnv(req *http.Request) map[string]string { + scriptName, pathInfo := p.resolveScriptName(req.URL.Path) + + https := "" + if req.TLS != nil { + https = "On" + } + remoteAddr := req.Header.Get("X-Client-IP") remotePort := "" if remoteAddr == "" { diff --git a/local/php/envs_test.go b/local/php/envs_test.go index ac9ad28f..34b2df87 100644 --- a/local/php/envs_test.go +++ b/local/php/envs_test.go @@ -182,6 +182,94 @@ func (s *PHPFPMSuite) TestGenerateEnv(c *C) { "SCRIPT_NAME": "/index.php", }, }, + { + passthru: "/index.php", + uri: "/subdirectory", + expected: map[string]string{ + "PATH_INFO": "", + "REQUEST_URI": "/subdirectory", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/index.php", + }, + }, + { + passthru: "/index.php", + uri: "/subdirectory/", + expected: map[string]string{ + "PATH_INFO": "/", + "REQUEST_URI": "/subdirectory/", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/index.php", + }, + }, + { + passthru: "/index.php", + uri: "/subdirectory/unknown.php", + expected: map[string]string{ + "PATH_INFO": "/unknown.php", + "REQUEST_URI": "/subdirectory/unknown.php", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/index.php", + }, + }, + { + passthru: "/index.php", + uri: "/subdirectory/unknown.php/", + expected: map[string]string{ + "PATH_INFO": "/unknown.php/", + "REQUEST_URI": "/subdirectory/unknown.php/", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/index.php", + }, + }, + { + passthru: "/index.php", + uri: "/subdirectory/index.php/foo", + expected: map[string]string{ + "PATH_INFO": "/foo", + "REQUEST_URI": "/subdirectory/index.php/foo", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/index.php", + }, + }, + { + passthru: "/index.php", + uri: "/subdirectory/subdirectory/", + expected: map[string]string{ + "PATH_INFO": "/", + "REQUEST_URI": "/subdirectory/subdirectory/", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/subdirectory/index.php", + }, + }, + { + passthru: "/index.php", + uri: "///subdirectory", + expected: map[string]string{ + "PATH_INFO": "", + "REQUEST_URI": "///subdirectory", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/index.php", + }, + }, + { + passthru: "/index.php", + uri: "/subdirectory///subdirectory//foo/", + expected: map[string]string{ + "PATH_INFO": "//foo/", + "REQUEST_URI": "/subdirectory///subdirectory//foo/", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/subdirectory/index.php", + }, + }, } for _, test := range tests { process := &Server{ @@ -197,7 +285,7 @@ func (s *PHPFPMSuite) TestGenerateEnv(c *C) { for k, v := range test.expected { vv, ok := env[k] c.Assert(ok, Equals, true) - c.Assert(vv, DeepEquals, v) + c.Assert(vv, DeepEquals, v, Commentf("#test uri:\"%s\" varName:\"%s\"", test.uri, k)) } } } diff --git a/local/php/testdata/public/subdirectory/index.php b/local/php/testdata/public/subdirectory/index.php new file mode 100644 index 00000000..e69de29b diff --git a/local/php/testdata/public/subdirectory/subdirectory/index.php b/local/php/testdata/public/subdirectory/subdirectory/index.php new file mode 100644 index 00000000..e69de29b From 1f434ad59613954d33e0145fbb529bb7c08d055d Mon Sep 17 00:00:00 2001 From: maciekpaprocki Date: Sat, 25 Feb 2023 00:50:04 +0000 Subject: [PATCH 2/5] added fix for loading php files outside of document root --- local/php/envs.go | 34 +++++++++++++++++++++++++----- local/php/envs_test.go | 41 ++++++++++++++++++++++++++++++++---- local/php/testdata/index.php | 0 3 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 local/php/testdata/index.php diff --git a/local/php/envs.go b/local/php/envs.go index d0bf19b5..0df041b0 100644 --- a/local/php/envs.go +++ b/local/php/envs.go @@ -32,22 +32,44 @@ import ( func (p *Server) resolveScriptName(pathInfo string) (string, string) { if pos := strings.Index(strings.ToLower(pathInfo), ".php"); pos != -1 { file := pathInfo[:pos+4] - if _, err := os.Stat(filepath.Join(p.documentRoot, file)); err == nil { - return file, pathInfo[pos+4:] + if file == filepath.Clean(file) { + if _, err := os.Stat(filepath.Join(p.documentRoot, file)); err == nil { + return file, pathInfo[pos+4:] + } } } + // quick return if it's short or path starts with // + if len(pathInfo) <= 1 || pathInfo[0:2] == "//" { + return p.passthru, pathInfo + } + + // removes first slash to make sure we don't loop through it as it always need to be there. + paths := strings.Split(pathInfo[1:], "/") - paths := strings.Split(strings.Trim(pathInfo, "/"), "/") for n := len(paths); n > 0; n-- { pathPart := paths[n-1] if pathPart == "" { continue } - file := filepath.Join(append(paths[:n], p.passthru)...) + // we on purpose don't use filepath join as it resolves the paths. This way if clean filepath is different we break + folder := string(filepath.Separator) + strings.Join(paths[:n], string(filepath.Separator)) + + if folder != filepath.Clean(folder) { + continue + } + + file := filepath.Join(folder, p.passthru) + path := strings.Join(paths[n:], "/") + if _, err := os.Stat(filepath.Join(p.documentRoot, file)); err == nil { - return "/" + file, pathInfo[strings.LastIndex(pathInfo, pathPart)+len(pathPart):] + // I am not sure how we can get rid of this if statements. It's complete abomination, but it's because subdirectory and subdirectory/ should go to this same file, but have different pathinfo + if path == "" && pathInfo[len(pathInfo)-1:] != "/" { + return file, "" + } + return file, "/" + path } + } return p.passthru, pathInfo @@ -56,6 +78,8 @@ func (p *Server) resolveScriptName(pathInfo string) (string, string) { func (p *Server) generateEnv(req *http.Request) map[string]string { scriptName, pathInfo := p.resolveScriptName(req.URL.Path) + //fmt.Println(req.URL.Path + " | " + scriptName + " | " + pathInfo + " | " + filepath.Clean(scriptName)) + https := "" if req.TLS != nil { https = "On" diff --git a/local/php/envs_test.go b/local/php/envs_test.go index 34b2df87..c4a710cc 100644 --- a/local/php/envs_test.go +++ b/local/php/envs_test.go @@ -252,20 +252,53 @@ func (s *PHPFPMSuite) TestGenerateEnv(c *C) { passthru: "/index.php", uri: "///subdirectory", expected: map[string]string{ - "PATH_INFO": "", + "PATH_INFO": "///subdirectory", "REQUEST_URI": "///subdirectory", "QUERY_STRING": "", - "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/index.php", - "SCRIPT_NAME": "/subdirectory/index.php", + "SCRIPT_FILENAME": testdataDir + "/public/index.php", + "SCRIPT_NAME": "/index.php", }, }, { passthru: "/index.php", uri: "/subdirectory///subdirectory//foo/", expected: map[string]string{ - "PATH_INFO": "//foo/", + "PATH_INFO": "///subdirectory//foo/", "REQUEST_URI": "/subdirectory///subdirectory//foo/", "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/index.php", + }, + }, + { + passthru: "/index.php", + uri: "/../index.php", + expected: map[string]string{ + "PATH_INFO": "/../index.php", + "REQUEST_URI": "/../index.php", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/index.php", + "SCRIPT_NAME": "/index.php", + }, + }, + { + passthru: "/index.php", + uri: "/subdirectory/../../index.php", + expected: map[string]string{ + "PATH_INFO": "/../../index.php", + "REQUEST_URI": "/subdirectory/../../index.php", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/index.php", + }, + }, + { + passthru: "/index.php", + uri: "/subdirectory/subdirectory/foo/subdirectory/bar", + expected: map[string]string{ + "PATH_INFO": "/foo/subdirectory/bar", + "REQUEST_URI": "/subdirectory/subdirectory/foo/subdirectory/bar", + "QUERY_STRING": "", "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/subdirectory/index.php", "SCRIPT_NAME": "/subdirectory/subdirectory/index.php", }, diff --git a/local/php/testdata/index.php b/local/php/testdata/index.php new file mode 100644 index 00000000..e69de29b From caacbd16ad7af131908119fbc8bef81fcbe28a99 Mon Sep 17 00:00:00 2001 From: maciekpaprocki Date: Sat, 25 Feb 2023 00:50:42 +0000 Subject: [PATCH 3/5] removed forgotten debug statement --- local/php/envs.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/local/php/envs.go b/local/php/envs.go index 0df041b0..775003c8 100644 --- a/local/php/envs.go +++ b/local/php/envs.go @@ -78,8 +78,6 @@ func (p *Server) resolveScriptName(pathInfo string) (string, string) { func (p *Server) generateEnv(req *http.Request) map[string]string { scriptName, pathInfo := p.resolveScriptName(req.URL.Path) - //fmt.Println(req.URL.Path + " | " + scriptName + " | " + pathInfo + " | " + filepath.Clean(scriptName)) - https := "" if req.TLS != nil { https = "On" From 3d76f9f2f6a1fc7ea0a4cfc7d0a709b679d56270 Mon Sep 17 00:00:00 2001 From: maciekpaprocki Date: Sat, 25 Feb 2023 01:22:40 +0000 Subject: [PATCH 4/5] added check for resolution with direct file --- local/php/envs_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/local/php/envs_test.go b/local/php/envs_test.go index c4a710cc..f9733b06 100644 --- a/local/php/envs_test.go +++ b/local/php/envs_test.go @@ -303,6 +303,17 @@ func (s *PHPFPMSuite) TestGenerateEnv(c *C) { "SCRIPT_NAME": "/subdirectory/subdirectory/index.php", }, }, + { + passthru: "/index.php", + uri: "/foo/../update.php", + expected: map[string]string{ + "PATH_INFO": "/foo/../update.php", + "REQUEST_URI": "/foo/../update.php", + "QUERY_STRING": "", + "SCRIPT_FILENAME": testdataDir + "/public/index.php", + "SCRIPT_NAME": "/index.php", + }, + }, } for _, test := range tests { process := &Server{ From 150e5ec1cb18bf7474321b28a93995965e2fc543 Mon Sep 17 00:00:00 2001 From: maciekpaprocki Date: Fri, 10 Mar 2023 15:24:07 +0000 Subject: [PATCH 5/5] started rewrite --- local/php/envs.go | 33 +++++++++++---------------------- local/php/envs_test.go | 8 ++++---- 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/local/php/envs.go b/local/php/envs.go index 775003c8..1f1a0957 100644 --- a/local/php/envs.go +++ b/local/php/envs.go @@ -23,6 +23,7 @@ import ( "net" "net/http" "os" + "path" "path/filepath" "strings" @@ -30,6 +31,9 @@ import ( ) func (p *Server) resolveScriptName(pathInfo string) (string, string) { + + pathInfo = path.Clean(pathInfo) + if pos := strings.Index(strings.ToLower(pathInfo), ".php"); pos != -1 { file := pathInfo[:pos+4] if file == filepath.Clean(file) { @@ -38,36 +42,21 @@ func (p *Server) resolveScriptName(pathInfo string) (string, string) { } } } - // quick return if it's short or path starts with // - if len(pathInfo) <= 1 || pathInfo[0:2] == "//" { + + if len(pathInfo) <= 1 { return p.passthru, pathInfo } - // removes first slash to make sure we don't loop through it as it always need to be there. - paths := strings.Split(pathInfo[1:], "/") + paths := strings.Split(pathInfo, "/") for n := len(paths); n > 0; n-- { - pathPart := paths[n-1] - if pathPart == "" { - continue - } - - // we on purpose don't use filepath join as it resolves the paths. This way if clean filepath is different we break - folder := string(filepath.Separator) + strings.Join(paths[:n], string(filepath.Separator)) - - if folder != filepath.Clean(folder) { - continue - } - - file := filepath.Join(folder, p.passthru) - path := strings.Join(paths[n:], "/") - + file := filepath.Clean(filepath.Join(paths[:n]...)) if _, err := os.Stat(filepath.Join(p.documentRoot, file)); err == nil { - // I am not sure how we can get rid of this if statements. It's complete abomination, but it's because subdirectory and subdirectory/ should go to this same file, but have different pathinfo - if path == "" && pathInfo[len(pathInfo)-1:] != "/" { + serverPath := path.Join(paths[n:]...) + if serverPath == "" && pathInfo[len(pathInfo)-1:] != "/" { return file, "" } - return file, "/" + path + return file, "/" + serverPath } } diff --git a/local/php/envs_test.go b/local/php/envs_test.go index f9733b06..8927f4cc 100644 --- a/local/php/envs_test.go +++ b/local/php/envs_test.go @@ -263,11 +263,11 @@ func (s *PHPFPMSuite) TestGenerateEnv(c *C) { passthru: "/index.php", uri: "/subdirectory///subdirectory//foo/", expected: map[string]string{ - "PATH_INFO": "///subdirectory//foo/", - "REQUEST_URI": "/subdirectory///subdirectory//foo/", + "PATH_INFO": "/subdirectory/foo/", + "REQUEST_URI": "/subdirectory/subdirectory/foo/", "QUERY_STRING": "", - "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/index.php", - "SCRIPT_NAME": "/subdirectory/index.php", + "SCRIPT_FILENAME": testdataDir + "/public/subdirectory/subdirectory/index.php", + "SCRIPT_NAME": "/subdirectory/subdirectory/index.php", }, }, {