Fix base URL bug #8508
Fix base URL bug #8508
Conversation
|
Please fix all the cs issues. |
|
Does this break things for people running laravel in a folder? |
|
Sorry about the code style. Should be good now... It does not break things for running it in a sub directory. (At least it didn't for me) |
| protected function prepareBaseUrl() | ||
| { | ||
| $scriptName = $this->server->get('SCRIPT_NAME'); | ||
| if(starts_with(ltrim($this->server->get('REQUEST_URI'), '/'), ltrim($scriptName, '/'))) |
There was a problem hiding this comment.
Where exactly? Are there any more specific style guidelines regarding spacing? (other than PSR-1 and that bit in the docs)
There was a problem hiding this comment.
before the opening bracket just after the if.
|
I've fixed all the coding style issues (I hope) and also fixed a little bug for generating URLs when running under root on windows (strangely |
|
Thanks! |
|
Please squash to one commit. |
|
I actually can't replicate this bug anyway. StyleCI runs laravel 5.0, and https://styleci.io/123/donate shows 404 correctly. It looks like it's caused by server missconfiguration, rather than a bug with laravel. |
|
You have to actually put |
|
Oh right. My bad. |
|
Then send a PR to Symfony. |
|
Hmm. In an effort to do that I just discovered that the symfony site itself handles this case correctly. |
|
symfony.com is running drupal though, so it's not totally using the same routing system. |
|
Ahh well that's an explanation ;) |
|
symfony.com is not running on drupal. At least there is no indicator for this. Why wouldn't it run on symfony? |
|
HEAVILY NSFW Only site i can think of http://www.youporn.com/app/app.php But it does do work as this bug suggests. |
|
@aequasi Haha I definitely won't use that example in a bug report / PR... |
|
Sorry to interrupt your discussion, but regarding this comment:
I'm afraid that's not true. I'm the guy in charge of symfony.com development, so I can assure you that the site is developed using the latest stable version of the Symfony full-stack framework. |
|
@javiereguiluz You're not interrupting at all. Thanks for clearing that up. I guess my next step will be setting up a symfony application myself to do some testing... |
|
set up a quick app on Heroku for the PR, using the demo app. Should be pretty easy |
|
@aequasi Sure, just thought it would be nice to demonstrate the bug on a big site... |
|
@lukasgeiter Here's a fun example. |
|
http://laravel.com/docs/5.0/yeah/this/might/be/a/symphony/bug/but/meh/index.php/docs/5.0/controllers Maybe you should think about shortening Larvel's urls? ;D |
|
I did a issue for this on Symphony, seems there is a PR to fix the issue hopefully will get merged. Haven't tested if it fixes the issue yet symfony/symfony#13617 pr: symfony/symfony#14335 |
|
@garygreen I've just tried the PR and it didn't change anything... I also found a live example: http://cmf.symfony.com/foo/bar/app.php/news |
|
Created a PR: symfony/symfony#14468 Let's see how that goes... |
This solves the issue #8441
It might not be considered as a real bug, but the Symfony Request makes a mistake when determining the base URL of a request.
If the application is running under domain root, following URLs will all produce the same path info:
foo/barcan be replaced with anything, the page will still work and not return a 404.For a live example hit: http://laravel.com/foo/index.php/docs/5.0/
This PR fixes that behavior but I'd really like some feedback from you guys:
prepareBaseUrl()is a lot more complicated. What do you think?