[ticket/11215] Correct paths when path info is used for controller access #1102

imkingdavid · 2012-11-20T00:15:33Z

http://tracker.phpbb.com/browse/PHPBB3-11215

bantu · 2012-11-20T00:22:17Z

phpBB/includes/functions.php

-	$server_parameters['REQUEST_URI'] = $request_uri . $path_info;
-	$server_parameters['SCRIPT_NAME'] = '';
+	$path = '';
+	for ($i = 0; $i < $corrections; $i++)


str_repeat() ?

imkingdavid · 2012-11-20T00:39:44Z

Accidentally closed it, it's still open.

imkingdavid · 2012-11-23T21:37:29Z

@igorw @p @naderman
This patch adds a function that calculates the relative path from the given URL to the board web root. That path is then prepended to all template root path variables in page_header() as well as all links generated by append_sid().

This is perhaps not the most elegant solution, but it is the best I could come up with, and I have tested it and it works. If anyone has any suggestions for something else, let me know; otherwise, this can be merged so that controllers can return to using path_info instead of the query string.

bantu · 2012-11-25T23:43:33Z

phpBB/includes/functions.php

+		return $path;
+	}
+
+	$corrections = substr_count($path_info, '/');


What about http://localhost/phpBB/./././?

Any number of /././././... is automatically removed by the browser. At least, it is for me.

Also, I just noticed this function https://github.com/phpbb/phpbb3/blob/develop/phpBB/includes/functions.php#L1055 which can be used to clean the path if needed.

p · 2012-11-26T08:57:28Z

Since I have not seen an explanation for how you arrived at this solution:

Research which environment variables php uses and passes to the userland. In case of an apache module, php does not use environment variables on input but uses apache apis to take the request apart. In case of cgi/fastcgi, php uses environment variables in particular path_info/script_uri/script_name. Then, php passes some/all of path_info/script_uri/script_name to the userland, regardless of sapi being used.
php knows which part of the uri requested is for the physical filename and which part is for the "virtual path". It has to in order to output said filename to the browser. Research this for all sapis. You'll have to read php source as I don't think this is completely documented in user documentation.
Create a table of all environment variables that are passed by php to userland with what is in them (physical path/virtual path/nothing), for each sapi.

Then we can start to think about how a valid solution might be implemented.

imkingdavid · 2013-02-26T23:15:58Z

@p @igorw @naderman To bump this, I understand that this is not the most elegant solution. I'm just highly doubtful that we will find a very good solution that isn't going to completely redesign how we handle pathing.

I have yet to find any issues with this patch, and am willing to work further on making it better, but I don't have the time or interest to go as in depth as was described in the previous comment. If someone would like to provide a better solution, they are welcome to. Until then, I'm convinced that a hacky solution is what we're going to have to live with for the time being. I would much rather have a hacky solution that works for 3.1 than wait until 4.0 to get better paths.

p · 2013-02-27T00:26:36Z

Can you write tests for apache/mod_php, cgi and fastcgi that cover the functionality being discussed here?

imkingdavid · 2013-02-28T19:49:27Z

@p How do I specify certain tests for certain servers?

p · 2013-02-28T20:12:07Z

I do not think this is covered by phpbb tests so you would need to implement it.

p · 2013-03-03T08:52:00Z

Implementing these tests in wolis should be straightforward (but not trivial, it will still take some effort). If you want to pursue this option I can help you with it.

imkingdavid · 2013-03-03T19:53:33Z

I have very little familiarity with python, but am definitely willing to learn to use it if you don't mind helping me some. I'll take a look at wolis and we can go from there.

imkingdavid · 2013-04-19T21:25:49Z

@p To bump this, I did find this function http://php.net/manual/en/function.php-sapi-name.php that might come in handy. I don't think I understand how tests on different sapi's would differ, though.

p · 2013-04-19T21:48:53Z

http environment variables may be set differently.

…cess PHPBB3-11215

PHPBB3-11215

When Symfony Request calculates path info, both of the following URLs give "/" as the path info: ./app.php and ./app.php/ This commit ensures that the proper correction is made. PHPBB3-11215

* develop: (176 commits) [feature/bootstrap-dic] Bootstrap container from config.php [ticket/11548] Use new static methods for request and submit [ticket/10772] Updating tests [ticket/10772] Remove 3.1 code [ticket/11388] Add newlines at EOF [ticket/11388] INCLUDECSS [ticket/11548] Run array_map on complete error array and not just colour_error [ticket/11644] Skip phpbb_dbal_order_lower_test on MySQL 5.6 [ticket/11388] Do not append assets_version if using remote path (e.g. http) [ticket/11388] Fix typo [ticket/11388] Remove typehints (causing tests to fail) [ticket/11388] Disable cache if IN_INSTALL defined [ticket/11388] Do not modify by reference [ticket/11388] typehits [ticket/11388] INCLUDEJS test for //(url) [ticket/11388] INCLUDEJS supports //(url) [ticket/11388] Fixing includejs test [ticket/11388] includejs inherit from includeasset [feature/twig] Unit tests for includejs [ticket/8319] Add migration file for update change ...

PHPBB3-11215

* develop: (53 commits) [ticket/11671] Update composer.lock [ticket/11671] Update composer.lock [ticket/11671] Add phing as a dependency and upgrade deps [ticket/11668] Move lint test to the end for travis [ticket/11626] Remove last reference to template in ldap [ticket/11626] Remove LDAP dependency on template [develop-olympus] Increment version number to 3.0.13-dev. [develop-olympus] Add changelog for 3.0.12 release. [develop-olympus] Bump version numbers for 3.0.12-RC1 release. [develop-olympus] Bumping version numbers to final for 3.0.12 releases. [ticket/11669] Fix PHP bug #55124 (recursive mkdir on /./) [ticket/11668] Run lint test at the end of the test suite [ticket/11548] Fix test errors in groups test on develop [ticket/11548] Check upload avatar URL the same way as in phpBB 3.0 [ticket/11548] Fix incorrect usage of array_map on acp groups page [ticket/11665] Fix test class name [ticket/11664] Stop creating php.html file in root path in tests [ticket/11665] Can't change file names already sent to set_filenames [ticket/11662] Typos: occured -> occurred [ticket/11662] Typos: occured -> occurred ...

PHPBB3-112515

* develop: [ticket/11647] Always use & for URLs [ticket/11647] Allow custom ports [ticket/11647] Use $config for assets_version [ticket/11647] Fix invalid variable name [ticket/11647] New assets handling [ticket/11647] Fix tests for INCLUDEJS

* develop: (214 commits) [ticket/10999] Fix assets_version in ACP [prep-release-3.0.12] More changelog items for the 3.0.12 release. [ticket/11687] Add assets_version to phpbb_config [ticket/11670] Consistency with logo: Replace "phpBB(tm)" with "phpBB(R)". [ticket/11674] Do not include vendor folder if there are no dependencies. [ticket/11685] Remove logout confirmation page [ticket/11684] Remove useless confirmation page after login and admin login [ticket/9657] Define user before injecting [ticket/9657] Remove old code [ticket/9657] Keep approval state of posts/topics when copying them [ticket/9657] Add functional tests for forking a copy [ticket/9657] Add functional test for splitting topic [ticket/11112] Do not change opensource.org link to https [ticket/9657] Add functional test for restoring a post/topic [ticket/11112] Use https for user-visible links to phpbb.com [ticket/9657] Fix a little error when moving softdeleted topics [ticket/9657] Add unit tests for softdeleting and moving posts/topics [ticket/9657] Remove last references to m_restore permission [ticket/9657] Fix english language :( [ticket/9657] Notifications do not require emails or jabber anymore ...

* develop: [ticket/11675] Fix template loop [ticket/11690] Old module class names may get autoloaded by class_exists [ticket/9649] Display information on index for moderators on unapproved posts [ticket/11686] Not checking for phpBB Debug errors on functional tests [ticket/11553] Typo [ticket/11553] Replace bullet with unicode [ticket/11553] Move bulletin points to pseudo class [ticket/11600] Remove duplicate test case [ticket/11600] Use lowercase null and remove duplicate test cases [ticket/11600] Use local variable for $user in test_localize_errors [ŧicket/11600] Split get driver tests into tests for all and only enabled ones [ticket/11600] Increase code test coverage of avatar manager

* develop: [ticket/11694] Do not locate assets with root path

PHPBB3-11215

In tests, the old app.php?controller=foo structure was used. Instead it should be app.php/foo. PHPBB3-11215

PHPBB3-11215

Because we check to see if the rewrite module is available we don't have to comment the rewrite lines PHPBB3-11215

PHPBB3-11215

EXreaction · 2013-09-02T22:32:24Z

phpBB/.htaccess

@@ -1,12 +1,34 @@
+<IfModule mod_rewrite.c>
+#
+# Uncomment the following line if you will be using any of the URL


comment no longer necessary

PHPBB3-11215

EXreaction · 2013-09-02T22:48:55Z

Looks good to me

[ticket/11215] Correct paths when path info is used for controller access

nickvergessen · 2013-09-03T08:15:07Z

I'm missing tests for app.php ?

It still needs to generate such URLs?

EXreaction · 2013-09-03T14:42:29Z

What tests are you missing and what URLs are you talking about?

nickvergessen · 2013-09-03T16:12:47Z

urls with app.php?controller= when the php module is disabled?

EXreaction · 2013-09-03T16:28:18Z

I'm assuming you mean with the apache module mod-rewrite? The urls are now phpbb.com/app.php/some/path when the rewriting is disabled. Mod rewrite isn't required for this functionality.

Urls using rewriting will be phpbb.com/some/path

bantu reviewed Nov 20, 2012
View reviewed changes

imkingdavid closed this Nov 20, 2012

imkingdavid reopened this Nov 20, 2012

bantu reviewed Nov 25, 2012
View reviewed changes

imkingdavid added 13 commits July 6, 2013 23:20

[ticket/11215] Correct paths when path info is used for controller ac…

c20f92b

…cess PHPBB3-11215

[ticket/11215] A few minor optimizations for phpbb_get_web_root_path()

0f522dd

PHPBB3-11215

[ticket/11215] Correct for different URL but same path info

b9c290b

When Symfony Request calculates path info, both of the following URLs give "/" as the path info: ./app.php and ./app.php/ This commit ensures that the proper correction is made. PHPBB3-11215

[ticket/11215] use global

3a87a6b

PHPBB3-11215

[ticket/11215] Don't try to correct paths during tests

068d350

PHPBB3-11215

[ticket/11215] Don't try to use when it isn't there

fab7f5f

PHPBB3-112515

Merge branch 'develop' into ticket/11215

3be753c

* develop: [ticket/11647] Always use & for URLs [ticket/11647] Allow custom ports [ticket/11647] Use $config for assets_version [ticket/11647] Fix invalid variable name [ticket/11647] New assets handling [ticket/11647] Fix tests for INCLUDEJS

Merge branch 'develop' into ticket/11215

9485eea

* develop: [ticket/11694] Do not locate assets with root path

Merge branch 'develop' into ticket/11215

af1eb74

imkingdavid added 10 commits August 31, 2013 14:31

[ticket/11215] Everything appears to be working...

ffee476

PHPBB3-11215

[ticket/11215] Use new URL structure for controllers

73859da

In tests, the old app.php?controller=foo structure was used. Instead it should be app.php/foo. PHPBB3-11215

Merge remote-tracking branch 'upstream/develop' into ticket/11215

1b1dd01

[ticket/11215] Add commented-out URL rewrite capability to .htaccess

9d48ee4

PHPBB3-11215

[ticket/11215] Make controller helper url() method use correct format

913e5a1

PHPBB3-11215

[ticket/11215] Uncomment rewrite rules in .htaccess

1e64095

Because we check to see if the rewrite module is available we don't have to comment the rewrite lines PHPBB3-11215

[ticket/11215] Update comment in .htaccess

5c50dc7

PHPBB3-11215

[ticket/11215] Add newline back to .htaccess, fix wording

5ed5f43

PHPBB3-11215

[ticket/11215] Fix helper_url_test.php tests

a1b4c6f

PHPBB3-11215

Merge branch 'develop' into ticket/11215

a057349

EXreaction reviewed Sep 2, 2013
View reviewed changes

[ticket/11215] Remove unnecessary comment

918ffc1

PHPBB3-11215

EXreaction added a commit that referenced this pull request Sep 2, 2013

Merge pull request #1102 from imkingdavid/ticket/11215

4fd99a7

[ticket/11215] Correct paths when path info is used for controller access

EXreaction merged commit 4fd99a7 into phpbb:develop Sep 2, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ticket/11215] Correct paths when path info is used for controller access #1102

[ticket/11215] Correct paths when path info is used for controller access #1102

imkingdavid commented Nov 20, 2012

bantu Nov 20, 2012

imkingdavid commented Nov 20, 2012

imkingdavid commented Nov 23, 2012

bantu Nov 25, 2012

imkingdavid Feb 13, 2013

imkingdavid Mar 13, 2013

p commented Nov 26, 2012

imkingdavid commented Feb 26, 2013

p commented Feb 27, 2013

imkingdavid commented Feb 28, 2013

p commented Feb 28, 2013

p commented Mar 3, 2013

imkingdavid commented Mar 3, 2013

imkingdavid commented Apr 19, 2013

p commented Apr 19, 2013

EXreaction Sep 2, 2013

EXreaction commented Sep 2, 2013

nickvergessen commented Sep 3, 2013

EXreaction commented Sep 3, 2013

nickvergessen commented Sep 3, 2013

EXreaction commented Sep 3, 2013

[ticket/11215] Correct paths when path info is used for controller access #1102

[ticket/11215] Correct paths when path info is used for controller access #1102

Conversation

imkingdavid commented Nov 20, 2012

bantu Nov 20, 2012

Choose a reason for hiding this comment

imkingdavid commented Nov 20, 2012

imkingdavid commented Nov 23, 2012

bantu Nov 25, 2012

Choose a reason for hiding this comment

imkingdavid Feb 13, 2013

Choose a reason for hiding this comment

imkingdavid Mar 13, 2013

Choose a reason for hiding this comment

p commented Nov 26, 2012

imkingdavid commented Feb 26, 2013

p commented Feb 27, 2013

imkingdavid commented Feb 28, 2013

p commented Feb 28, 2013

p commented Mar 3, 2013

imkingdavid commented Mar 3, 2013

imkingdavid commented Apr 19, 2013

p commented Apr 19, 2013

EXreaction Sep 2, 2013

Choose a reason for hiding this comment

EXreaction commented Sep 2, 2013

nickvergessen commented Sep 3, 2013

EXreaction commented Sep 3, 2013

nickvergessen commented Sep 3, 2013

EXreaction commented Sep 3, 2013