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.

Sign up for GitHub

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

Jump to bottom

Incompatibility with composer ~1.3.0 and setup:di:compile #8635

Closed
AntonEvers opened this issue Feb 21, 2017 · 5 comments
Closed

Incompatibility with composer ~1.3.0 and setup:di:compile #8635

AntonEvers opened this issue Feb 21, 2017 · 5 comments
Assignees
@orlangur
Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed

Comments

@AntonEvers
Copy link
Contributor

Preconditions

  1. Install composer 1.3.0 or higher
  2. Check out Magento 2.0.10

Steps to reproduce

  1. run composer update
  2. run bin/magento setup:di:compile-multi-tenant -vvv

Expected result

  1. compilation runs fine

Actual result

[Magento\Framework\Exception\LocalizedException]
Source class "\Magento\Catalog\Model\Product\OptionInterface" for "\Magento\Catalog\Model\Product\OptionInterfacePersistor" genera
tion does not exist.

I've added the solution to this in https://magento.stackexchange.com/questions/147219/optioninterfacepersistor-error-when-compiling-magento-in-production-mode/161010#161010

I will create a pull request for you to fix this.
This was referenced Feb 21, 2017
Closed
Closed
Closed
@orlangur
Copy link
Contributor

orlangur commented Feb 21, 2017 •
edited
Loading

run composer update

Does not seem to be a totally safe command to me. Will ./vendor/bin/composer update do the trick?

As to me, it's better to have Composer 1.3.x completely supported (see #4359) than do some magic like this one.

@AntonEvers
Copy link
Contributor Author

Hello @orlangur,

I wasn't aware of ticket #4359.
Let's keep this ticket pending until #4359 is fixed indeed.

As to the recommended composer executable, http://devdocs.magento.com/guides/v2.1/install-gde/prereq/integrator_install_composer.html will lead more developers to using composer 1.3.x.

In my case ./vendor/composer/composer/bin/composer update certainly does the trick. ./vendor/bin/composer update gives me errors.

PHP Warning:  require(/var/www/vhosts/magento2/httpdocs/vendor/bin/../src/bootstrap.php): failed to open stream: No such file or directory in /var/www/vhosts/magento2/httpdocs/vendor/bin/composer on line 8

Using ./vendor/composer/composer/bin/composer looks like a promising workaround for now.

@orlangur
Copy link
Contributor

@ajpevers, yeah, I checked existing documentation before writing comment and there is no suggestion to use composer from vendor.

Thanks, good to know that ./vendor/bin/composer is not working.

The bad thing with relying on unstable version is that in some cases both of them could not work. I faced with such issues when adding new requirements - in some Composer versions there were bugs with version contraint resolution.

@AntonEvers
Copy link
Contributor Author

@orlangur no problem. I'll manage with the workarounds. Let's focus on #4359 and we'll be relieved of these problems 👍 . Thanks for your help.

@ktomk ktomk mentioned this issue Mar 1, 2017
Merged
ktomk added a commit to ktomk/magento2 that referenced this issue Mar 1, 2017
@ktomk
Validate PHP classnames in di.xml files via schema
81e3b67 
The preferenceType @for/@type attributes, the typeType @name attribute,
the virtualTypeType @type attribute and the pluginType @type attribute
contain class-names (FQCNs) which should not start with a leading
backslash (U+005C "\") and should not contain other invalid character
sequences that would represent an invalid PHP class-name.

Previously this was possible and these errors got unnoticed within di.xml
configuration file validation.

The ObjectManager - a user of these configurations - handles this common
error in user input in part so far by removing any trailing slashes with
multiple calls like:

        $type = ltrim($type, '\\');

This change has been introduced in 33ebc24 and could be classified as a
workaround for a quite common mistake when specifying an FQCN that despite
the varying notations in the PHP manual due to the contextual resolution
rules (and different to a file-systems absolute path in POSIX) must not
start with a leading separator as type or class-name.

When using a string-value as class-name (e.g. class_exists() calls) the
class name is always an FQCN as namespacing in PHP is contextual within
source-code for identifiers only and not for strings.

So relative class-names (like those with a leading backslash) do not
apply for strings. This is the case in configuration files like di.xml
files. The di.xml files use the

    urn:magento:framework:ObjectManager/etc/config.xsd

schema location which is resolved by UrnResolver (6379732) to

    lib/internal/Magento/Framework/ObjectManager/etc/config.xsd

That schema did validate class-name attribute values only against the
simple type of being strings (xs:string). As a class-name in PHP is a
valid string also if starting with the backslash character (and other
invalid names, like digits in front), this commit extends the schema
to validate against the regular expression provided by the PHP manual [1]:

    ^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$

by adding a new simple-type called "phpFqcn" that qualifies the string-
base-type with the from PHP manual translated pattern:

    [a-zA-Z_-ÿ][a-zA-Z0-9_-ÿ]*

extended for namespaced class-names:

    ([a-zA-Z_-ÿ][a-zA-Z0-9_-ÿ]*)(\\[a-zA-Z_-ÿ][a-zA-Z0-9_-ÿ]*)*

The change ensures that the said attribute values are validated and
invalid class-names are recognized during schema based validation.

This change prevents that configured PHP-types can be autoloaded when
used w/o smudging (see the ltrim() operation). It has been documented [2]
that the leading backslash prevents correct file-resolution when auto-
loading with the Composer autoloader, a component actively used by the
Magento application.

This change adheres to existing PR magento#8638 and relates to issue magento#8635.

Refs:

- magento#8635

- magento#8638

- [1] https://php.net/language.oop5.basic

- [2] http://magento.stackexchange.com/a/161010

- 33ebc24

- 6379732
ktomk added a commit to ktomk/magento2 that referenced this issue Mar 2, 2017
@ktomk
Validate PHP class-names in di.xml files via schema
a207012 
The preferenceType @for/@type attributes, the typeType @name attribute,
the virtualTypeType @type attribute and the pluginType @type attribute
contain class-names (FQCNs) which should not start with a leading
backslash (U+005C "\") and should not contain other invalid character
sequences that would represent an invalid PHP class-name.

Previously this was possible and these errors got unnoticed within di.xml
configuration file validation.

The ObjectManager - a user of these configurations - handles this common
error in user input in part so far by removing any trailing slashes with
multiple calls like:

        $type = ltrim($type, '\\');

This change has been introduced in 33ebc24 and could be classified as a
workaround for a quite common mistake when specifying an FQCN that despite
the varying notations in the PHP manual due to the contextual resolution
rules (and different to a file-systems absolute path in POSIX) must not
start with a leading separator as type or class-name.

When using a string-value as class-name (e.g. class_exists() calls) the
class name is always an FQCN as namespacing in PHP is contextual within
source-code for identifiers only and not for strings.

So relative class-names (like those with a leading backslash) do not
apply for strings. This is the case in configuration files like di.xml
files. The di.xml files use the

    urn:magento:framework:ObjectManager/etc/config.xsd

schema location which is resolved by UrnResolver (6379732) to

    lib/internal/Magento/Framework/ObjectManager/etc/config.xsd

That schema did validate class-name attribute values only against the
simple type of being strings (xs:string). As a class-name in PHP is a
valid string also if starting with the backslash character (and other
invalid names, like digits in front), this commit extends the schema
to validate against the regular expression provided by the PHP manual [1]:

    ^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$

by adding a new simple-type called "phpClassName" that qualifies the string-
base-type with the from PHP manual translated pattern:

    [a-zA-Z_-ÿ][a-zA-Z0-9_-ÿ]*

extended for namespaced class-names:

    ([a-zA-Z_-ÿ][a-zA-Z0-9_-ÿ]*)(\\[a-zA-Z_-ÿ][a-zA-Z0-9_-ÿ]*)*

The change ensures that the said attribute values are validated and
invalid class-names are recognized during schema based validation.

This change verifies that configured PHP-types can be autoloaded when
used w/o smudging (see the ltrim() operation). It has been documented [2]
that the leading backslash prevents correct file-resolution when auto-
loading with the Composer autoloader, a component actively used by the
Magento application.

This change adheres to existing PR magento#8638 and relates to issue magento#8635.

Additionally this commit adds tests for the new phpClassName xsd type with
config xml material.

Refs:

- magento#8635

- magento#8638

- [1] https://php.net/language.oop5.basic

- [2] http://magento.stackexchange.com/a/161010

- 33ebc24

- 6379732
@magento-engcom-team magento-engcom-team added G1 Passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@orlangur
Copy link
Contributor

orlangur commented Oct 8, 2017 •
edited
Loading

@ajpevers, thanks for reporting, issue seems to be fixed on 2.2.0 branch.

@orlangur orlangur closed this as completed Oct 8, 2017
@orlangur orlangur added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Oct 8, 2017
@orlangur orlangur self-assigned this Oct 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@orlangur orlangur

Labels
Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@orlangur @AntonEvers @magento-engcom-team