8000 Index.php in subdirectories [ feature request ] · Issue #237 · symfony-cli/symfony-cli · GitHub
[go: up one dir, main page]
Skip to content

Index.php in subdirectories [ feature request ] #237

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.

Sign up for GitHub

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

Jump to bottom
Open
maciekpaprocki opened this issue Dec 6, 2022 · 13 comments
Open

Index.php in subdirectories [ feature request ] #237

maciekpaprocki opened this issue Dec 6, 2022 · 13 comments

Comments

@maciekpaprocki
Copy link

First of all, big fan of this tool. It cuts a lot of loading time on mac vs docker and it's very easy to set up. I am pretty much recommending it to everyone who listens ( and a lot of people who don't :) ).

And here's a problem. Your project is too good to support only modern PHP frameworks. It would be great for it to be able to support WP too as it really requires such a tool. Fortunately, it's really not far from accomplishing that.

Currently, WP pretty much works out of the box for the front end. It struggles a bit on the backend though. The issue is that all the requests will always go to the index.php in the web unless file.php is appended.

Example web folder structure ( using bedrock, composer version of WP ):

web
  - index.php
  - wp
    - wp-admin
      - index.php

When going to / I see web/index.php
When going to /wp/wp-admin/index.php I see web/wp/wp-admin/index.php
When going to /wp/wp-admin/ I weirdly see web/index.php again.

Allowing the directories paths to go to the root index in them would solve this issue. I think it might be breaking change, but it would be weird that someone would have other files and didn't want to serve them, so maybe it's not.

This change would probably make behaviour more similar to how apache and Nginx work by default, so possible it would make other old-school frameworks work out of the box.

I am happy to help with some coding but probably will have to be pointed in the right direction as Go is not my strong suit.
@tucksaun
Copy link
Member
tucksaun commented Dec 6, 2022

Let me rephrase your need to be clear: when the requested URL is a directory, the Symfony CLI should check for an index.php file, and if one is found use it as the passthru controller, otherwise fallback to the default passthru controller?

If so, I see the use case and it might be easy enough to make it work.
But we might also encounter more edge cases as we need to support FPM but also PHP built-in server.
And what if one does not want this behavior because they are migrating some legacy code exposing PHP scripts to Symfony?
Apache and Nginx have a configurability we don't have to allow users to override this behavior so we have to think about where to draw the line.

@fabpot any thoughts about this?

@maciekpaprocki
Copy link
Author

Yes, that's exactly it.

I kind of assumed that everyone is using the server only locally. If not then yes, that might have security implications. That could be specified as a parameter then or YAML config and maybe even disabled by default.

If it's used locally only then I don't really see much of a problem with even legacy code. They probably want it to run either way. Also, that's probably a more common configuration. From my experience either it's htaccess / nginx working exactly like that or Nginx just hardcoding everything to one index.php.

I understand the configurability point. I am not sure if the plan is for symfony-cli to not stay only symfony / modern tool, and that would be completely fair. Said that console commands were a good gateway drug for me, maybe symfony-cli will be for some other devs. WP also seriously needs some alternatives to Xamp or docker.

@mwansinck
Copy link
mwansinck commented Dec 21, 2022 •
edited
Loading

Seems you try to use Symfony CLI with WordPress in a bedrock directory style;
Our solution to solve this issue is as following (add those 3 lines after <?php in the web/index.php file.

#./web/index.php
if ('/' !== $_SERVER['PATH_INFO'] && file_exists($_SERVER['DOCUMENT_ROOT'] . $_SERVER['PATH_INFO'] . 'index.php')) {
	include $_SERVER['DOCUMENT_ROOT'] . $_SERVER['PATH_INFO'] . 'index.php'; exit;
}

this code detects if an index.php should be accessed, includes it and terminates the current index.php after that.

@maciekpaprocki
Copy link
Author

I am using something similar, but I would prefer not to have to deploy that to prod. If I have the wrong server configuration I prefer it to fail than to realise in half a year it's using different file. Also, seems like this is the only thing that makes it not work out of box.

In the code above, I would sanitise path. Probably not large risk, but better safe than sorry.

@mwansinck
Copy link

I am using something similar, but I would prefer not to have to deploy that to prod. If I have the wrong server configuration I prefer it to fail than to realise in half a year it's using different file. Also, seems like this is the only thing that makes it not work out of box.

Good point, I added an check if it's running with Symfony Server, so it won't have any affect on production. It's indeed the only think that doesn't work, so for us the workaround is acceptabel.

if (str_starts_with( $_SERVER['SERVER_SOFTWARE'], 'Symfony Local Server') && '/' !== $_SERVER['PATH_INFO'] && file_exists( $_SERVER['DOCUMENT_ROOT'] . $_SERVER['PATH_INFO'] . 'index.php' ) ) {
	include $_SERVER['DOCUMENT_ROOT'] . $_SERVER['PATH_INFO'] . 'index.php';
	exit;
}

I also tried to create a solution with a php.ini and auto_prepend_file but run into the issue that the path should be relative (otherwise it has to updated for each developer / project path).

In the code above, I would sanitise path. Probably not large risk, but better safe than sorry.

Good point also, although the above change would make this unnecessary, because the risk is only on development (when running Symfony Server)

@maciekpaprocki
Copy link
Author

@tucksaun, I am thinking of trying PR for this, but I need to know how configurable it should be.

Few questions:

  1. Should it be configurable at all or just always on?
    I don't think there are any security implications if cli is always used on local
  2. If configurable, should it be in YAML or cli arguments?
  3. If configurable, can it be 'on' by default?

@mwansinck
Copy link

@tucksaun, I am thinking of trying PR for this, but I need to know how configurable it should be.

Few questions:

  1. Should it be configurable at all or just always on?
    I don't think there are any security implications if cli is always used on local
  2. If configurable, should it be in YAML or cli arguments?
  3. If configurable, can it be 'on' by default?

I believe always on should be fine and doesn't add new security implications. By adding the index.php explicitly to the url the same url can be requested, so doing this automatically no pages can be reached that where not reachable before. Also webservers like nginx do support the same kind of behavior, so applications (like WordPress) should already take care of unwanted direct access to subdirectory (index.)php files. Last Symfony CLI should only be used local..

@tucksaun
Copy link
Member

Given the fact that we already handle any PHP file if the suffix is here I'm okay with checking if there's an index.php present and if so process it as if the URL is explicit.
This is the extension of an already present feature so I would not make this configurable (at least yet).

But I would only limit this to index.php and to me this is the limit we should draw.
If one needs more rewrite rules as Nginx or Apache provides, they need to use them or we need to think about this properly about how to implement it (and find the time because it would be non-trivial)

a couple of pointers:

  • you will need to work in github.com/symfony-cli/symfony-cli/local/http#Server.Handler
  • I suggest adding some tests for this Handler first. thanks to the fact that this handler is only calling a Callback it should not be too complicated only to test the logic about whether the PHP is being processed or not

@maciekpaprocki maciekpaprocki mentioned this issue Feb 1, 2023
Open
@cjhaas
Copy link
cjhaas commented Feb 2, 2023

Very excited for this! We are a WordPress/Drupal/Symfony shop, and we use the Symfony CLI for proxy and serving for all of these frameworks locally. I can't tell you how much dev-ops work this has saved, especially since we've got Windows, WSL and Mac for development.

The "index.php" has been the only annoying part, and I'll be glad when that is solved.

@YetiCGN
Copy link
YetiCGN commented Mar 6, 2023 •
edited
Loading

This will also help us run TYPO3 via Symfony Local Web Server, which is currently not possible in 11.0+ due to a new backend rewrite.

@ceesvanegmond
Copy link

Actually having the same issue; would be great if we can think of an fix

@maciekpaprocki
Copy link
Author

I am gonna get back to it one day. The feature was almost ready but got stuck on reviews and a rewrite would be good.

@nlemoine
Copy link
nlemoine commented Jan 20, 2024 •
edited
Loading

For those of you using WordPress, I just released a package that applies a couple of fixes to make symfony-cli work smoother on local env: https://github.com/nlemoine/wp-symfony-local-server

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@YetiCGN @tucksaun @ceesvanegmond @maciekpaprocki @nlemoine @cjhaas @mwansinck
0