georust / gpx

Rust read/write support for GPS Exchange Format (GPX)
https://crates.io/crates/gpx
MIT License
98 stars 44 forks source link

fix: no longer crash on extensions-ception #82

Closed ndrsg closed 1 year ago

ndrsg commented 1 year ago

fix handling of extensions-tags inside extensions-tags by counting the depth

lnicola commented 1 year ago

Can you please squash if it's not too much of a hassle?

ndrsg commented 1 year ago

sure :)

urschrei commented 1 year ago

Just want to note that I very much enjoyed "extensions-ception"

lnicola commented 1 year ago

I can't check right now, but can there be two closing tags for only one opening tag? What happens in that case?

ndrsg commented 1 year ago

1 opening followed by 2 closing tags:

This extensions-consume function would exit on the first closing tag without error.

This would lead to the outer logic to receive a extensions-closing tag. Depends if this case is handled there.

Might happen with other non-gpx-tags inside an extensions-block as well.

lnicola commented 1 year ago

But there's no code to return out of the function when the depth reaches 0. The old version used to do that.

ndrsg commented 1 year ago

I will have a look tomorrow.

Another option might be recursion instead of counting depth?

ndrsg commented 1 year ago

soo.. I changed the approach a bit and oriented on the other implementations, also keeping in mind a future implementation of parsing the extensions content into a structure with kind of "generic parser"?

also added some tests, where I would expect errors. turned out, that the xmlreader already tests the xml-structure quite well.

now treating "extensions"-tags inside an "extensions"-block just like any other tag, hence its just coincidence, if an internal tag is called "extensions", right?