agsh / onvif

ONVIF node.js implementation
http://agsh.github.io/onvif/
MIT License
693 stars 236 forks source link

Issue #134 Improve xmlns parsing #135

Closed bl0ggy closed 4 years ago

bl0ggy commented 4 years ago

Permit to remove xmlns in the Dialect of a PullMessagesResponse like the following:

<wsnt:Topic Dialect="http://www.onvif.org/ver10/tev/topicExpression/ConcreteSet xmlns:wsnt=http://docs.oasis-open.org/wsn/b-2 xmlns:tns1=http://www.onvif.org/ver10/topics xmlns:tnssamsung=http://www.samsungcctv.com/2011/event/topics">tns1:Device/tns1:Trigger/tnssamsung:DigitalInput</wsnt:Topic>
coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 87.306% when pulling e916241ef529cb9cb7939ab40129695276d6090e on bl0ggy:Issue#134 into fb421bdc5f8523641538cba677e0e9e3168f950d on agsh:master.

RogerHardiman commented 4 years ago

Thanks for the pull request. Just out of interest which camera needed the change?

UPDATE - I see it is a Samsung cameras. I've not tested against Samsung so thank you for your contribution

RogerHardiman commented 4 years ago

Help. I'm trying to test the regex change using the online RegEx tester at regex101.com and also in jsconsole. I'm not sure how your new new regex works.

You are trying to remove xmlns from the following

tns1:Device/tns1:Trigger/tnssamsung:DigitalInput

I noticed the xmlns entries do not have Quote Marks (speech marks) in then. Normally we have xmlns:xxxx="text_in_quotes" whereas this XML has xmlns:xxx=text_with_no_quotes

The regex is still looking for the quotes.

xml.replace(/xmlns([^=]?)=(".?")/g,'')

So I'm not sure how it works.

Now it may be that I'm just reading it wrong so could you help with some extra info please?

bl0ggy commented 4 years ago

Sure, The important thing to note in the old Regex is that the (.*) accepts any character. In the case of the following XML

<Parent xmlns:ns1="url1">
    <Child Dialect="xmlns:ns2=url2"/>
</Parent>
<Parent xmlns:ns3="url3">
    <Child Dialect="xmlns:ns4=url4"/>
</Parent>

the old Regex will match xmlns:ns1 correctly with :ns1 as (.?), however, when coming to xmlns:ns2, (.?) will match all characters until it find exactly =", which is in xmlns:ns3, so

:ns2=url2"/>
</Parent>
<Parent xmlns:ns3

while the new Regex accepts all characters that are before =" excepted = signs, so it does not match xmlns:ns2=url2 as the = sign is not followed by a ".

As such, xmlns:ns=url won't be replaced as it does not match.

RogerHardiman commented 4 years ago

Thankyou for the explanation. Commit merged and thankyou for taking the time to find the bug and fix it and submit a PR.

I had assumed the regex was trying to remove namespaces from the Topic Dialect string too. That is what confused me.