stackery / php-lambda-layer

PHP Runtime Layer for AWS Lambda
Other
320 stars 70 forks source link

Value of PHP_SELF when router script is in task subdirectory #11

Closed andybee closed 5 years ago

andybee commented 5 years ago

In our projects the index.php is typically in a src folder inside the task. We've set the handler as src/index.php and this successfully executes with the PHP layer.

However, when PHP's built-in web server is run and the router is supplied in a sub-directory (e.g. php -S 0:8080 src/index.php), the value of PHP_SELF becomes the path of the URL called, rather than /src/index.php/foo/bar etc.

If you run the code from the root (e.g. php -S 0:8080 index.php) this problem does not exist.

I'm not sure if this behaviour is intentional or not, and that ultimately it could be a PHP issue rather than the layer, but currently it breaks the behaviour of popular routing engines such as Slim as they attempt to strip PHP_SELF from the path before evaluating routes and, therefore, fall over when it's value is actually the path.

Just wondering if anyone else had any better insight on this problem?

On way it could be resolved, and how I'm tempted to fix it up in our codebase for now, is to tweak the behaviour of bootstrap so that it can detect a subdirectory in _HANDLER and change to that path to run the script from PWD.

txase commented 5 years ago

As the current maintainer of the layer, I just want to say that I don't know what the right resolution is to this issue, and I probably am not the best person to figure it out. I'll be looking for someone to make a PR with a cogent thesis of why and how the behavior should change.

🙏

atrope commented 5 years ago

@andybee I Had a similar problem with Phalcon Framework, i had to change its Router to use the URI Source to the Request URI in Phalcon: $router->setUriSource(\Phalcon\Mvc\Router::URI_SOURCE_SERVER_REQUEST_URI); Maybe you should check with Slim Documentation if maybe they also have this option.

andybee commented 5 years ago

I've submitted a pull request which fixes this issue in the layer, but I'm also going to try and raise the issue with the PHP team as I don't understand why the value differs that way with the internal web server.

If it's intentional behaviour, we need to consider the importance of this layer using the internal web server. If you run PHP under FPM etc. as a FastCGI process, this issue doesn't exist, so do we wish to replicate this behaviour, or specifically the behaviour of the internal web server?

Out of interest, why is the internal web server being utilised over spawning a standard PHP process, for example? Is it because of the long running nature? Could we use php-cgi process similarly? That's the approach I took with my pre-layers approach to executing PHP in a Lambda function.

andybee commented 5 years ago

I've reported the issue on the PHP bug tracker too in case I'm missing an intentional reason it's this way: https://bugs.php.net/bug.php?id=77253

txase commented 5 years ago

@andybee awesome, thanks for all your help tracking this down!

To answer your question at the end, there are issues with each approach I investigated for implementing PHP:

php-cgi (and any other possible web server PHP mechanisms) would work as well, and would be interesting to compare :).

barryvdh commented 5 years ago

Here are some comparisons using different methods, including the built-in server: https://github.com/mnapoli/bref-bootstrap-benchmarks

I created a PR using php-cgi, which allows to detect if the .sock file is created. Not sure if it fixes any other issues though (or creates new ones), but hot performance seems similar to the built-in server, coldstart is faster (because you don't have to wait 1 sec): https://github.com/mnapoli/bref-bootstrap-benchmarks/pull/7

barryvdh commented 5 years ago

Also see this comment about checking the webserver status: https://github.com/mnapoli/bref-bootstrap-benchmarks/issues/10#issuecomment-445453011

atrope commented 5 years ago

@barryvdh You did a great job! I managed to, based on your code, to update the code used in stackery's bootsrap.

Per my tests It is Working, better than the CLI directly. https://github.com/SuaMusica/php-lambda-layer/blob/master/bootstrap This is the build(I Also install phalcon but you can remove should you want): https://github.com/SuaMusica/php-lambda-layer/blob/master/build.sh

Thanks for the great work a will continue here trying to fix some issues. I Am having an issue that some AJAX requests(Posts with data) are not even getting to lambda on requests through ALB. Without data it works like a charm, and with data it also works if not AJAX..

So By Now, everything that gets to Lambda is working through APIGW or ALB.

If you want you can try the layer here:

arn:aws:lambda:us-east-1:272581579102:layer:php71:146

barryvdh commented 5 years ago

Probably some of the headers are net sent correctly, like the content type or something. But nice :)

txase commented 5 years ago

@barryvdh Awesome work factoring through all the possible implementations!

I know that any of the other implementation types would be a departure from the initial implementation, but would you be open to collaborating to select the best solution and get that merged into this project? Looks like PHP-FPM might be a great solution :).

Thanks!

andybee commented 5 years ago

As I suspect would happen, no response from the PHP community on this one.

It's intentionally referred to by the PHP community as a development aide rather than an actual production-grade way of servicing HTTP requests with the PHP runtime and as the discussion has moved on I suspect we're favouring a move to php-cgi or php-fpm anyway, which alleviates this issue.

Tempted to close this one and suggest those working on an FPM or CGI variant open a new issue to more formerly propose that?

barryvdh commented 5 years ago

This seems more active in regards with community effort: https://github.com/mnapoli/bref

txase commented 5 years ago

As there hasn't been any progress on this issue and there's no clear sense that the layer is behaving improperly, I'm going to close this issue. Please feel free to re-open if you believe the layer still is behaving incorrectly.

Thanks!

andybee commented 5 years ago

The layer is behaving correctly assuming you the goal it to behave like the PHP CLI tool. If the goal is to act like PHP under a CGI environment, it's not behaving correctly and the way it acts breaks several popular PHP router libraries. Hope that helps make an informed decision for the project.

txase commented 5 years ago

That's true, and the goal is for the layer to behave like a PHP in a CGI environment. I suppose this issue falls in the category of: "it's not technically wrong, but it still breaks everything."

I'll re-open this issue. I won't be able to develop a solution for this issue anytime soon, though. PRs are welcome :).

Thanks!

andybee commented 5 years ago

I submitted #12 as a way to "fix" the behaviour back at the start of December.

txase commented 5 years ago

Ugh, GitHub got the best of me again... I know issues and PRs are intermixed (they share the same number sequence), so when I go through all the issues I forget to check the PRs independently. I also just failed in spinning my context back up after the initial flurry of development activity.

Given what you've found from upstream and what you've noted from experience trying to use the layer with popular frameworks, I think your solution makes sense. I'll review and merge it.

I appreciate your support, and I'm sorry for the delay. The PR just slipped my mind and I hadn't remembered nor noticed it in some time.

andybee commented 5 years ago

No worries, I appreciate your efforts and the challenges of managing an OSS project.

txase commented 5 years ago

I just pushed a new version of the layer (version 7) that includes the fix for this issue (#12).

Thanks again!