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.

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

Infinite loop in elementMap handler if not parsing inner elements #70

Closed
judgej opened this issue Dec 17, 2015 · 2 comments
Closed

Infinite loop in elementMap handler if not parsing inner elements #70

judgej opened this issue Dec 17, 2015 · 2 comments

Comments

@judgej
Copy link

judgej commented Dec 17, 2015

This is my XML fragment:

        <despatchNote ID="383265">
          <detail ID="1" isbn="9781909633773" Qty="1" Status="OK" UMC="1.86"></detail>
          <detail ID="2" isbn="9781909633896" Qty="1" Status="--- Book not in Catalogue --" UMC="0"></detail>
        </despatchNote>

I want to turn each detail tag into a DespatchNoteLine object. The following goes into an infinite loop, creating DespatchNoteLine objects until the request runs out of memory.

        $reader->elementMap = [
            '{}detail' => function ($reader) {
                $attributes = $reader->parseAttributes();
                return new DespatchNoteLine($attributes);
            },
        ...

I get around this by parsing the inner elements (even though there are no inner elements):

        $reader->elementMap = [
            '{}detail' => function ($reader) {
                $attributes = $reader->parseAttributes();
                \Sabre\Xml\Element\Base::xmlDeserialize($reader); // <<<=== The fix.
                return new DespatchNoteLine($attributes);
            },
        ...

Is this expected behaviour? Should it be necessary to always explicitly parse or "consume" inner elements? My impression is that not parsing what's inside the detail elements would simply discard what's in there. But perhaps I'm looking at it wrong - the parser will parse everything and the mapping is a layer that sits on top of that.

Any thoughts?

@evert
Copy link
Member

evert commented Dec 17, 2015

Yes you absolutely have to advance the reader, because the reader doesn't have a way to know what your intent is.

Thankfully it's pretty easy. Instead of \Sabre\Xml\Element\Base::xmlDeserialize($reader);, just call $reader->next();

Leaving this open as a documentation bug.

@judgej
Copy link
Author

judgej commented Dec 17, 2015

That works great - thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants