Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

URI path present in _GET / Input::get() in case of PHP7+fpm #2167

Open
bartlomiejb opened this issue Dec 28, 2021 · 6 comments
Open

URI path present in _GET / Input::get() in case of PHP7+fpm #2167

bartlomiejb opened this issue Dec 28, 2021 · 6 comments

Comments

@bartlomiejb
Copy link

bartlomiejb commented Dec 28, 2021

After migrating our apps to a newer server software we noticed that there is an URI path in _GET now sometimes (when there is no query params in URL proper).

For example lets modify an example initial Fuel app (welcome.php controller):

	public function action_hello()
	{
		//return Response::forge(Presenter::forge('welcome/hello'));
		echo '<pre><tt>hello' . "\n\n";print_r([$_GET, Input::get()]);die();
	}

After visiting http://domain/hello we get:

hello

Array
(
    [0] => Array
        (
            [/hello] => 
        )

    [1] => Array
        (
            [/hello] => 
        )

)

The culprit is the rule in .htaccess:
https://github.com/fuel/fuel/blob/1.9/develop/public/.htaccess#L66

RewriteRule ^(.*)$ index.php?/$1 [QSA,L]

I guess the fix for this should be in Fuel core somewhere... Or maybe you have another idea?

From the application programmer point of view it shoud work just the same as it was the case of another server software (eg. PHP5 or PHP7+sapi), ie. no "garbage" in _GET :-)

@bartlomiejb bartlomiejb changed the title URI always path present in _GET / Input::get() in case of PHP7+fpm URI path always present in _GET / Input::get() in case of PHP7+fpm Dec 28, 2021
@bartlomiejb bartlomiejb changed the title URI path always present in _GET / Input::get() in case of PHP7+fpm URI path present in _GET / Input::get() in case of PHP7+fpm Dec 28, 2021
@WanWizard
Copy link
Member

Indeed, it should have been caught.

WanWizard added a commit to fuel/fuel that referenced this issue Dec 28, 2021
@bartlomiejb
Copy link
Author

bartlomiejb commented Dec 30, 2021

Thanks for a quick response!

fuel/fuel@60e4690
Is this the proper fix?
When pointing to this rewrite rule as a culprit I was mainly thinking about an unnecessary question mark (?).

Anyway, removing it is fixing the issue for me (in 1.8)... I was trying it before, but due to some configuration option on our server it didn't work...

@WanWizard
Copy link
Member

It works in 8.0 too.

@bartlomiejb
Copy link
Author

Maybe I was not clear enough in above comment, so here is full explanation:
by saying "removing it" I meant by "it" the question mark, NOT "QSA".

Removing "QSA" doesn't change anything in regard of URI path in GET

What fixes the issue for me (in Fuel release version) is removing the question mark from the rewrite rule, ie.:
with this rule URI path is present in _GET and Input::get():

RewriteRule ^(.*)$ index.php?/$1 [L]

with this rule URI path is NOT present in _GET and Input::get():

RewriteRule ^(.*)$ index.php/$1 [L]

BTW: in current Fuel dev the URI path is still present in Input::get() (but not in raw _GET). (Removal of the question mark from the rewrite rule fixes it.)


By "Fuel release" I mean code installed by instructions from https://fuelphp.com/docs/installation/instructions.html#/from_github - "Clone the latest release from github", git+composer version.

By "Fuel dev" I mean code installed by instructions from https://fuelphp.com/docs/installation/instructions.html#/from_github - "Clone the latest development branch from github", git+composer version.

Tested (among others) on current Arch Linux, PHP7 with FPM.

@WanWizard
Copy link
Member

WanWizard commented Dec 31, 2021

I need to know the exact server setup you test with.

My local test server runs PHP-FPM 8.0 on AlmaLinux, and for that setup, the rewrite rule used is

	# deal with php-fcgi first
	<IfModule mod_fcgid.c>
		RewriteRule ^(.*)$ index.php?/$1 [QSA,L]
	</IfModule>

and if I disable mod_cgi, the rule used is

			# for fpm installations
			<IfModule !mod_php7.c>
				RewriteRule ^(.*)$ index.php?/$1 [L,QSA]
			</IfModule>

Both work fine here, both $_GET and Input::get() are clean.

I tested with URL http://test.lab.local/test/test/index?var1=one&var2=two

But with or without question mark, with or without QSA, the result stays the same:
image

(I can't comment on the current release, as that might also have issues with the Input code)

@WanWizard WanWizard reopened this Dec 31, 2021
@bartlomiejb
Copy link
Author

I need to know the exact server setup you test with.

See https://github.com/bartlomiejb/for-fuel-core-issue-2167/tree/main/archlinux/etc
There are configuration files from ArchLinux (I only ommited /etc/httpd/modules - it is a symlink to directory with .so modules outside of /etc), the only changes from standard files as packaged in distro are my changes as described in https://wiki.archlinux.org/title/Apache_HTTP_Server#PHP (and for FPM) and added two local vhosts.

I installed packages for PHP7 as that's the version that I am interested in ;)

$ pacman -Q | grep -i 'apache\|fpm\|php'
apache 2.4.52-1
php7 7.4.27-1
php7-apache 7.4.27-1
php7-fpm 7.4.27-1
php7-sodium 7.4.27-1

With this config and these packages the results of print_r([$_GET, Input::get()]); are (when visiting <localhost>/hello):
Fuel Release:

Array
(
    [0] => Array
        (
            [/hello] => 
        )

    [1] => Array
        (
            [/hello] => 
        )

)

Fuel Dev:

Array
(
    [0] => Array
        (
        )

    [1] => Array
        (
            [/hello] => 
        )

)

My local test server runs PHP-FPM 8.0 on AlmaLinux (...)

Have you tried also with PHP7?

I tested with URL http://test.lab.local/test/test/index?var1=one&var2=two

Try omitting query params - from my tests on PHP7 the bug (ie. URI path) was not present when there were query params.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants