Parsing invalid XML doesn't throw an exception that can be caught

[dominickp]

When I use Reader and the KeyValue::xmlDeserialize with invalid XML, the KeyValue class seems to get hung up until it hits max_execution_time and throws a FatalErrorException.

I can't seem to figure out how to catch this FatalErrorException. I'd like a way to deal with invalid XML, because it happens from time-to-time and it currently locks up the application for a while, for the user.

Do you have any recommendations?

[evert]

Hi @dominickp ,

Could you share an example that triggers the problem? Would be very interesting to analyze. I was under the impression that all the built-in deserializers cannot trigger infinite loop conditions, but if this is not true, i would love to find out!

[dominickp]

Sure. Try this out:

protected function parseErrorExample(){

    $invalid_xml = '
        <Example Start="PRINTING" Prompt="YES" AutoClose="YES" SkipUnverified="NO" Test="NO">
            <Package ID="1">
                <MailClass>NONE</MailClass>
                <PackageType>ENVELOPE</PackageType>
                <WeightOz>1</WeightOz>
                <FleetType>DC</FleetType>
            <Package ID="2">
                <MailClass>NONE</MailClass>
                <PackageType>ENVELOPE</PackageType>
                <WeightOz>1</WeightOz>
                <FleetType>DC/FleetType>
            </Package>
        </Example>';
    $reader = new Reader();

    try{

        $reader->xml($invalid_xml);

        $reader->elementMap = [

            '{}Example' => function($reader) {
                $children = $reader->parseInnerTree();
                foreach($children as $child) {
                    if ($child['value'] instanceof Recipient) {
                        $this->recipients[] = $child['value'];

                    }
                }
                return $this->recipients;
            },

            '{}Package' => function($reader) {
                $recipient = new Recipient();
                // Borrowing a parser from the KeyValue class.
                $keyValue = KeyValue::xmlDeserialize($reader);

                if (isset($keyValue['{}WeightOz'])){
                    $recipient->setReferenceId($keyValue['{}WeightOz']);
                }

                return $recipient;
            },
        ];

        $parsed = $reader->parse();

    } catch(\Exception $e){
        throw new MyException(
            "Could not parse the document. Is the XML valid? Message: ".$e->getMessage()."."
        );

    }

}

One thing I learned while trying this example is, if I only use one Package, it catches an exception. If I use more than one (as in the example above) it times out and throws the following FatalErrorException which cannot be caught:

[evert]

Hi @dominickp ,

This is an interesting example, as the provided xml is actually invalid. The first package never gets a closing </Package>.

[dominickp]

@evert Good eye... did not mean to include that additional mistake. I meant only to include the issue with FleetType. Try with the following, it still reproduces the same error:

 $invalid_xml = '
            <Example Start="PRINTING" Prompt="YES" AutoClose="YES" SkipUnverified="NO" Test="NO">
                <Package ID="1">
                    <MailClass>NONE</MailClass>
                    <PackageType>ENVELOPE</PackageType>
                    <WeightOz>1</WeightOz>
                    <FleetType>DC</FleetType>
                </Package>
                <Package ID="2">
                    <MailClass>NONE</MailClass>
                    <PackageType>ENVELOPE</PackageType>
                    <WeightOz>1</WeightOz>
                    <FleetType>DC/FleetType>
                </Package>
            </Example>';

And yeah, it's invalid XML and should error out. The issue I'm having it I can't catch the exception and it seems the only way to prevent it is to alter max_execution_time.

[staabm]

FatalErrors can only be caught on PHP7, this is a php runtime limitation.

[evert]

Yea it all makes sense. It's sabre/xml job to detect the problem and do something with it.

I found the issue. We're calling XmlReader::read() at some point. It returns false and currently we're discarding that request. I think we need to find all instances where we call read() and throw an exception if it returned false.

[evert]

In particular it's this line:

https://github.com/fruux/sabre-xml/blob/master/lib/Reader.php#L145

Going to try to add an exception there, and hope that it doesn't have unintended side effects. Will be a bit later though.

[evert]

This is SOO weird. I created a unittest from your examples that reproduces it. But if I change anything to the test it throws an exception. Even if I'm making unrelated changes such as changing the element "Package" to "foo".

[evert]

Alright, I don't know why, but it seems that the loop only happens when the input xml is a certain size (in bytes). If the size is reduced enough, the behavior is correct. Odd issue, but should be able to work around it.