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

Move all "bootstrapping" code to bootstrap.php #371

Closed
ionas opened this issue Apr 23, 2016 · 9 comments
Closed

Move all "bootstrapping" code to bootstrap.php #371

ionas opened this issue Apr 23, 2016 · 9 comments
Assignees
Milestone

Comments

@ionas
Copy link
Contributor

ionas commented Apr 23, 2016

Could we move all this code:
https://github.com/cakephp/app/blob/master/webroot/index.php#L17-L26
and this code:
https://github.com/cakephp/app/blob/master/webroot/index.php#L28-L37

... to bootstrap.php?
This would mean:

  • built-in webserver detection is happening at the start of bootstrapping (and should/can be outcommented on most installations)
  • the dispatcher is happening at the end of bootstrapping (and could be swapped/changed etc)
@markstory
Copy link
Member

the dispatcher is happening at the end of bootstrapping (and could be swapped/changed etc)

Why can't it be swapped where it is?

@ionas
Copy link
Contributor Author

ionas commented Apr 23, 2016

The reason is basically that you would have control about the whole bootstrapping process within well.. bootstrap.php (or bootstrap.php loading dispatch.php or whatever at the bottom).

The index.php file would only be the entry point for the webserver and nothing else.

Edit: benefit would be that we can test for the PHP version before anything else happens (and for extensions).

@ionas
Copy link
Contributor Author

ionas commented Apr 23, 2016

It also means that if you upgrade cakephp you have one file less to worry about diffing/merging.

@ionas
Copy link
Contributor Author

ionas commented Apr 25, 2016

This was the issue that made me think that index.php should be dumb and bootstrap.php should do some checks at the top before anything else happens:
#370 (comment)

EDIT: if you agree, I would move things around in a test app, see if no issues arise and create a PR.

@ionas
Copy link
Contributor Author

ionas commented May 28, 2016

Shouldn't

// for built-in server
if (php_sapi_name() === 'cli-server') {
    $_SERVER['PHP_SELF'] = '/' . basename(__FILE__);
    $url = parse_url(urldecode($_SERVER['REQUEST_URI']));
    $file = __DIR__ . $url['path'];
    if (strpos($url['path'], '..') === false && strpos($url['path'], '.') !== false && is_file($file)) {
        return false;
    }
}

... now be in the middleware as one item (that can simply be disabled in Application.php)?

@markstory
Copy link
Member

I'm not sure it can entirely be middleware as it has to muck with the $_SERVER vars. I can look into whether or not the PSR7 lib we're using removes the need to include this though.

@markstory markstory added this to the 3.3.0 milestone May 28, 2016
@markstory markstory self-assigned this May 28, 2016
@markstory
Copy link
Member

Closing as I think this is 'done'. At the very least it is doable in applications where people want it to be done.

@ionas
Copy link
Contributor Author

ionas commented Jul 27, 2016

What about: https://github.com/cakephp/app/blob/3.next/webroot/index.php#L17-L26

// for built-in server
if (php_sapi_name() === 'cli-server') {
    $_SERVER['PHP_SELF'] = '/' . basename(__FILE__);
    $url = parse_url(urldecode($_SERVER['REQUEST_URI']));
    $file = __DIR__ . $url['path'];
    if (strpos($url['path'], '..') === false && strpos($url['path'], '.') !== false && is_file($file)) {
        return false;
    }
}
  • Can't this be moved to some Middleware that can be enabled/disabled.
  • If not, can it be moved to bootstrap?

@markstory
Copy link
Member

Can't this be moved to some Middleware that can be enabled/disabled.

Could but why?. Its only useful in the cli-server context, and having a middleware would mean it would be run on every request.

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