Make Magento compatible with composer ~1.3.0 #8638
Conversation
|
It would be great to see a regression test looking for these values. DI itself does ltrim from top of my head, scanning over the files w/ xpath and starts-with could do it. Should also help extension authors to check their XMLs. Small POC: https://gist.github.com/ktomk/a7690cc3353298eec609fdc1d76ad295 |
|
@ktomk, I remember leading backslash started to be trimmed at some point as such situation produced inconsistent behavior before that. As to me it would be better to be strict about that and simply disallow leading backslash. As this is a BC break it could be done in two phases: first deprecate the ability to use leading backslash and then remove in some future release. |
|
True, it makes sense to first deprecate this, I also thought about extension authors. IMHO there is nothing wrong to fix the |
|
And not to miss: Thanks for the initiative here, I had lost why it's not compatible with the stable composer version. ~~~That to say, a short explanation why this is necessary would be great to read~~~ /e: it's in the corresponding Magento Stackexchange answer. |
|
Fixing this repo is an easiest part, there are even some existing tests reading all active And also if we bump composer version in |
|
looks like we need to introduce static test (intergrity) which will cover these changes and prevent us from new occurrences of leading backslashes appear. |
|
@ktomk yes, this proposal sounds reasonable |
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
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
|
@orlangur Yes, due to your request, #8743 has 0b243b8 (as first commit, see https://github.com/magento/magento2/pull/8743/commits). I rebased it on this PR's branch. ;) |
|
Ah, okay. I think Github marks it as merged but now that you ask ... ;) |
Fixes #8635