Bugfix/app mage was not found

[eneiasramos]

Description (*)

Fixes the error message when trying to access index.php from a directory different than the root directory.

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes OpenMage/magento-lts#<issue_number>

Manual testing scenarios (*)

1. ...
2. ...

cd /tmp php /magento/root/directory/index.php

/tmp/app/Mage.php was not found

Questions or comments

Contribution checklist (*)

• [x] Pull request has a meaningful description of its purpose
• [x] All commits are accompanied by meaningful commit messages
• [x] All automated tests passed successfully (all builds are green)
• [ ] Add yourself to contributors list

[kiatng]

May I know the use case?

[fballiano]

wouldn't it be better to include app/Mage.php instead?

[eneiasramos]

The reason chdir() must remain in index.php is because on the next line:

define('MAGENTO_ROOT', getcwd());

The function getcwd() is setting MAGENTO_ROOT incorrectly.

[justinbeaty]

Or how about:

define('MAGENTO_ROOT', __DIR__);

But I also am curious what the use-case for this is.

[fballiano]

Or how about:

define('MAGENTO_ROOT', __DIR__);

I would prefer this solution a lot

[eneiasramos]

Or how about:

define('MAGENTO_ROOT', __DIR__);

I would prefer this solution a lot

This solution is better :)

[justinbeaty]

It seems getcwd() is used in some other places though, and some in lib/Zend which we cannot modify to use MAGENTO_ROOT since it will be a composer dependency soon.

https://github.com/OpenMage/magento-lts/search?q=getcwd

However I still am not sure when you would want to run index.php from the command line anyway...

[eneiasramos]

It seems getcwd() is used in some other places though, and some in lib/Zend which we cannot modify to use MAGENTO_ROOT since it will be a composer dependency soon.

https://github.com/OpenMage/magento-lts/search?q=getcwd

However I still am not sure when you would want to run index.php from the command line anyway...

so it makes sense to keep the chdir

[justinbeaty]

@eneiasramos Can you explain for what reason you are trying to execute index.php from a different directory? I am assuming no one has come across this issue because no one has tried to do this before.

If you're trying to run a custom script, it is usually done something like this:

define('MAGENTO_ROOT', '/magento/root/directory');

require MAGENTO_ROOT . '/app/bootstrap.php';
require_once MAGENTO_ROOT . '/app/Mage.php';

$scope = 'frontend'; // 'adminhtml';

Mage::app();
Mage::getConfig()->loadEventObservers($scope);
Mage::app()->addEventArea($scope);

[eneiasramos]

@justinbeaty

I don't have a specific reason. I just think this should run without any errors.

I found this error when I tried to run index.php from a different directory.

[justinbeaty]

@eneiasramos I just tried and even when I'm in the right directory, php index.php is just returning empty output. Which makes sense since there's no web request sent. I am not sure I see the point any more of this fix other than to prevent an error that nobody will see.

I am also against my suggestion to use __DIR__ because of functions like this which will break.

https://github.com/OpenMage/magento-lts/blob/f71321b37a782bd167ef56de0c6e18e67645685d/dev/tests/functional/tests/app/Mage/Adminhtml/Test/Constraint/AssertProductIsPresentOnCustomWebsite.php#L160-L171

[fballiano]

So I guess after @justinbeaty's last comment I think we should close this PR, sorry! :-(

[eneiasramos]

@justinbeaty what happens if you run index outside of public_html?

Ex:

php ./public_html/index.php

[eneiasramos]

@fballiano my suggestion is to use chdir instead of DIR

So I think the change is totally valid.

[justinbeaty]

@eneiasramos if I run it from another directory, I get the error as you described.

The resistance to this PR is why are we running index.php in the first place? There seems to be no reason to do this. All the chdir would do is add latency to each request (how much? I don’t know how fast or slow chdir is). And possibly add an unthought of vulnerability. All of this for no reason.

[fballiano]

Thanks for explaining it the way I would have liked to :-D

exactly, adding another file system operation in the main entry point is mmmm

[fballiano]

php ./public_html/index.php

why would somebody do that? It doesn't do anything, in 12 years of M1 I've never seen anybody run index.php file from CLI

[eneiasramos]

@fballiano hackers do this.

There's always a first time for everything.

[fballiano]

solved conflict, @kiatng @elidrissidev @sreichel @ADDISON74 what do you think about this?

[justinbeaty]

I still think it's unnecessary and a possible security issue we're overlooking. To avoid the error, what about skip the chdir and instead:

define('MAGENTO_ROOT', getcwd());

if(MAGENTO_ROOT !== __DIR__) {
    // maybe echo something...
    exit;
}

[addison74]

What is the use of this PR? Better said, who accesses index.php from another directory?

I don't know how beneficial the error suppression is in such a situation.

[elidrissidev]

I think this change is unnecessary. What's exactly the use case for running index.php from the cli in the first place?

[fballiano]

ok let's close it at the moment, sorry!

[addison74]

What @justinbeaty proposed above is a better implementation, but I don't see the usefulness of this PR, just not running index.php in the CLI?