search

magento-hackathon / magento-composer-installer

Composer installer for Magento modules
210 stars 154 forks source link

Specifying magento-root-dir should be optional #50

Closed joshribakoff closed 10 years ago

joshribakoff commented 10 years ago

Why do I have to specify the magento-root-dir? It should just default to ".", don't most people just put the composer.json in the project root?

Schrank commented 10 years ago

good question, I do it this way.

mmenozzi commented 10 years ago

Yes the composer.json is in the project root but Magento couldn't be in the root. Imho Magento shouldn't be in the project root because if you have composer.json and Magento at the same level you may have a security issues; you have to pay attention to what is accessible from the production webserver through HTTP requests.

For example, if you have Magento and composer.json in the project root and there isn't any special deny rule in your virtual host configuration, anyone could download your composer.json and know which dependencies you're using.

It's better to have Magento in a project subdir and have that subdir as a document root of the virtual host.

So, imho, Magento shouldn't be in the project root and putting . as magento-root-dir default value encourages a bad practice.

Bye.

Schrank commented 10 years ago

Then we put src as default or something else to encourage this :-) and we document it and write a warning and explanation

Von unterwegs gesendet

Am 12.10.2013 um 11:47 schrieb Manuele Menozzi notifications@github.com:

Yes the composer.json is in the project root but Magento couldn't be in the root. Imho Magento shouldn't be in the project root because if you have composer.json and Magento at the same level you may have a security issues; you have to pay attention to what is accessible from the production webserver through HTTP requests.

For example, if you have Magento and composer.json in the project root and there isn't any special deny rule in your virtual host configuration, anyone could download your composer.json and know which dependencies you're using.

It's better to have Magento in a project subdir and have that subdir as a document root of the virtual host.

So, imho, Magento shouldn't be in the project root and putting . as magento-root-dir default value encourages a bad practice.

Bye.

— Reply to this email directly or view it on GitHub.

joshribakoff commented 10 years ago

Not too big of a security problem to have composer.Jason visible but I can follow your logic. How about setting "public" as the default then? Or at least document these security concerns as was said On Oct 12, 2013 7:18 AM, "Fabian Blechschmidt" notifications@github.com wrote:

Then we put src as default or something else to encourage this :-) and we document it and write a warning and explanation

Von unterwegs gesendet

Am 12.10.2013 um 11:47 schrieb Manuele Menozzi notifications@github.com:

Yes the composer.json is in the project root but Magento couldn't be in the root. Imho Magento shouldn't be in the project root because if you have composer.json and Magento at the same level you may have a security issues; you have to pay attention to what is accessible from the production webserver through HTTP requests.

For example, if you have Magento and composer.json in the project root and there isn't any special deny rule in your virtual host configuration, anyone could download your composer.json and know which dependencies you're using.

It's better to have Magento in a project subdir and have that subdir as a document root of the virtual host.

So, imho, Magento shouldn't be in the project root and putting . as magento-root-dir default value encourages a bad practice.

Bye.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHubhttps://github.com/magento-hackathon/magento-composer-installer/issues/50#issuecomment-26195668 .

mmenozzi commented 10 years ago

Yes @Schrank, that's fine. I prefer web instead of src as default value because web better communicates the fact that web directory should be the document root directory. Look at Symfony2 framework, it uses web as document root directory. Of course, the real problem is that Magento itself doesn't care about having source code in the document root and encourage developers to put their code inside the document root.

@joshribakoff not too big of a security problem to have composer.json public? How can you say that? With the composer.json I have the list of your dependencies with related version. If there is an outdated dependency I can check for public known security issues of that dependency and use it to hack the application.

joshribakoff commented 10 years ago

Definitely a security concern but its not a big deal because even without composer.json your store is putting out this information already due to security problems in Magento itself, see this for example: http://www.whitefirdesign.com/tools/magento-version-check.html?

Also Zend Framework and a few other frameworks use "public" (zend framework 1 used "html"). I think most shared web hosts use "public_html". Maybe a default value doesn't make sense after all, because I do agree we shouldn't encourage people to put composer.json in the web root. It is a security concern, I agree.

mmenozzi commented 10 years ago

Yes @joshribakoff, Magento itself has this kind of problems. +1 for public or public_html as default value.

Schrank commented 10 years ago

or htdocs. If you ask me, we should use any standard some other framework uses.

web from symfony is a good choice if you ask me.

Vinai commented 10 years ago

So, if we are voting I'm all for htdocs

On the other hand, none of these defaults make a lot of sense to me since at least 90% (my guestimate) will be using something else, whatever is chosen.

Because of that a default makes no sense. A default only make sense if the majority will not have to change it.

If most people have to change it then having a default will lead to more confusion. I think it is better to maybe document the purpose of and the need to configure a root directory more prominently.

mmenozzi commented 10 years ago

I agree with @Vinai.

joshribakoff commented 10 years ago

Agreed with @Vinai

"." make sense since because I think 99% will put composer.json in the root (whether we like it or not). Its not a big deal to not have the default though if you think that encourages them.

On Mon, Oct 14, 2013 at 2:45 AM, Manuele Menozzi notifications@github.comwrote:

I agree with @Vinai https://github.com/Vinai.

— Reply to this email directly or view it on GitHubhttps://github.com/magento-hackathon/magento-composer-installer/issues/50#issuecomment-26238136 .

Josh Ribakoff www.vehiclefits.com 1-877-CAR-1788

Flyingmana commented 10 years ago

Yeah, maybe public composer.json is no security issue, a public vendor directory is.

To the issue:

  1. the installer has a strong effect to the chosen directory
  2. there is to much diversity in a public web directory

so, No, there will not be a default value for this. Iam not ready to accept responsibility for a destroyed non vcs project because of the chosen default.