Skip to content

Commit

Permalink
Validate PHP classnames in di.xml files via schema
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ktomk committed Mar 1, 2017
1 parent 0b243b8 commit 81e3b67
Showing 1 changed file with 19 additions and 5 deletions.
24 changes: 19 additions & 5 deletions lib/internal/Magento/Framework/ObjectManager/etc/config.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,20 @@
</xs:complexType>
</xs:redefine>

<xs:simpleType name="phpFqcn">
<xs:annotation>
<xs:documentation>
A string that matches a Fully Qualified Class Name from PHP, especially not starting
with a backslash as this is an invalid character to start a class name with but a
somewhat common mistake so this simple type can be used to validate against it
already
</xs:documentation>
</xs:annotation>
<xs:restriction base="xs:string">
<xs:pattern value="([a-zA-Z_&#x7f;-&#xff;][a-zA-Z0-9_&#x7f;-&#xff;]*)(\\[a-zA-Z_&#x7f;-&#xff;][a-zA-Z0-9_&#x7f;-&#xff;]*)*"/>
</xs:restriction>
</xs:simpleType>

<xs:complexType name="init_parameter">
<xs:complexContent>
<xs:extension base="argumentType" />
Expand Down Expand Up @@ -102,8 +116,8 @@
Preference help Object Manager to choose class for corresponding interface
</xs:documentation>
</xs:annotation>
<xs:attribute name="for" type="xs:string" use="required" />
<xs:attribute name="type" type="xs:string" use="required" />
<xs:attribute name="for" type="phpFqcn" use="required"/>
<xs:attribute name="type" type="phpFqcn" use="required" />
</xs:complexType>

<xs:complexType name="typeType">
Expand All @@ -121,7 +135,7 @@
</xs:element>
<xs:element name="plugin" type="pluginType" minOccurs="0" maxOccurs="unbounded" />
</xs:choice>
<xs:attribute name="name" type="xs:string" use="required" />
<xs:attribute name="name" type="phpFqcn" use="required" />
<xs:attribute name="shared" type="xs:boolean" use="optional" />
</xs:complexType>

Expand All @@ -133,14 +147,14 @@
With 'virtualType' tag you can point parameters and plugins for autogenerated class
</xs:documentation>
</xs:annotation>
<xs:attribute name="type" type="xs:string" use="optional" />
<xs:attribute name="type" type="phpFqcn" use="optional" />
</xs:extension>
</xs:complexContent>
</xs:complexType>

<xs:complexType name="pluginType">
<xs:attribute name="name" type="xs:string" use="required" />
<xs:attribute name="type" type="xs:string" use="optional" />
<xs:attribute name="type" type="phpFqcn" use="optional" />
<xs:attribute name="disabled" type="xs:boolean" use="optional" />
<xs:attribute name="sortOrder" type="xs:int" use="optional" />
</xs:complexType>
Expand Down

0 comments on commit 81e3b67

Please sign in to comment.