From 97caab0e7a40c4034888e3269cdcbb858e47b45b Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Mon, 7 Nov 2022 00:06:43 +0000 Subject: PHP: Fix a potential problem parsing the path. @dward on GitHub reported an issue with a URL like http://foo.bar/test.php?blah=test.php/foo where we would end up trying to run the script test.php?blah=test.php In the PHP module the format 'file.php/' is treated as a special case in nxt_php_dynamic_request() where we check the _path_ part of the url for the string '.php/'. The problem is that the path actually also contains the query string, thus we were finding 'test.php/' in the above URL and treating that whole path as the script to run. The fix is simple, replace the strstr(3) with a memmem(3), where we can limit the amount of path we use for the check. The trick here and what is not obvious from the code is that while path.start points to the whole path including the query string, path.length only contains the length of the _path_ part. NOTE: memmem(3) is a GNU extension and is neither specified by POSIX or ISO C, however it is available on a number of other systems, including: FreeBSD, OpenBSD, NetBSD, illumos, and macOS. If it comes to it we can implement a simple alternative for systems which lack memmem(3). This also adds a test case (provided by @dward) to cover this. Closes: Cc: Andrei Zeliankou Reviewed-by: Alejandro Colomar Reviewed-by: Andrei Zeliankou [test] Signed-off-by: Andrew Clayton --- test/test_php_targets.py | 1 + 1 file changed, 1 insertion(+) (limited to 'test/test_php_targets.py') diff --git a/test/test_php_targets.py b/test/test_php_targets.py index 918c5fda..eec1846f 100644 --- a/test/test_php_targets.py +++ b/test/test_php_targets.py @@ -47,6 +47,7 @@ class TestPHPTargets(TestApplicationPHP): assert self.get(url='/2')['body'] == '2' assert self.get(url='/blah')['status'] == 503 # TODO 404 assert self.get(url='/')['body'] == 'index' + assert self.get(url='/1.php?test=test.php/')['body'] == '1' assert 'success' in self.conf( "\"1.php\"", 'applications/targets/targets/default/index' -- cgit From 0be289b7fcd6c837710fb3868b3b3fbdc0395b64 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Thu, 26 Jan 2023 12:11:49 +0000 Subject: Tests: Add some PHP tests for 403 and 404 error handling. Since the previous commit, we now properly handle 403 Forbidden & 404 Not Found errors in the PHP language module. This adds a test for 403 Forbidden to test/test_php_application.py, but also fixes a test in test/test_php_targets.py where we were checking for 503 but should have been a 404, which we now do. Acked-by: Alejandro Colomar Cc: Andrei Zeliankou [ Incorporates a couple of small test cleanups from Andrei ] Signed-off-by: Andrew Clayton --- test/test_php_targets.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'test/test_php_targets.py') diff --git a/test/test_php_targets.py b/test/test_php_targets.py index eec1846f..2cf39c1c 100644 --- a/test/test_php_targets.py +++ b/test/test_php_targets.py @@ -45,7 +45,7 @@ class TestPHPTargets(TestApplicationPHP): assert self.get(url='/1')['body'] == '1' assert self.get(url='/2')['body'] == '2' - assert self.get(url='/blah')['status'] == 503 # TODO 404 + assert self.get(url='/blah')['status'] == 404 assert self.get(url='/')['body'] == 'index' assert self.get(url='/1.php?test=test.php/')['body'] == '1' -- cgit From 7934dcabbc3c2b585e8d3f8fcee7020ba26f1687 Mon Sep 17 00:00:00 2001 From: Andrei Zeliankou Date: Tue, 21 Feb 2023 17:21:29 +0000 Subject: Tests: switched to using f-strings. Previously, it was necessary to support older versions of Python for compatibility. F-strings were released in Python 3.6. Python 3.5 was marked as unsupported by the end of 2020, so now it's possible to start using f-strings safely for better readability and performance. --- test/test_php_targets.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'test/test_php_targets.py') diff --git a/test/test_php_targets.py b/test/test_php_targets.py index 2cf39c1c..e74f2ec6 100644 --- a/test/test_php_targets.py +++ b/test/test_php_targets.py @@ -6,6 +6,7 @@ class TestPHPTargets(TestApplicationPHP): prerequisites = {'modules': {'php': 'any'}} def test_php_application_targets(self): + targets_dir = f"{option.test_dir}/php/targets" assert 'success' in self.conf( { "listeners": {"*:7080": {"pass": "routes"}}, @@ -27,15 +28,15 @@ class TestPHPTargets(TestApplicationPHP): "targets": { "1": { "script": "1.php", - "root": option.test_dir + "/php/targets", + "root": targets_dir, }, "2": { "script": "2.php", - "root": option.test_dir + "/php/targets/2", + "root": f'{targets_dir}/2', }, "default": { "index": "index.php", - "root": option.test_dir + "/php/targets", + "root": targets_dir, }, }, } @@ -72,7 +73,7 @@ class TestPHPTargets(TestApplicationPHP): "targets": { "default": { "index": "index.php", - "root": option.test_dir + "/php/targets", + "root": f"{option.test_dir}/php/targets", }, }, } @@ -85,7 +86,7 @@ class TestPHPTargets(TestApplicationPHP): {"pass": "applications/targets/blah"}, 'listeners/*:7080' ), 'invalid targets pass' assert 'error' in self.conf( - '"' + option.test_dir + '/php/targets\"', + f'"{option.test_dir}/php/targets"', 'applications/targets/root', ), 'invalid root' assert 'error' in self.conf( -- cgit