kbr / fritzconnection

Python-Tool to communicate with the AVM Fritz!Box by the TR-064 protocol and the AHA-HTTP-Interface
MIT License
303 stars 59 forks source link

Escape illegal XML chars #115

Closed chemelli74 closed 2 years ago

kbr commented 2 years ago

As I can see you have started on this issue, like me; I have a slightly different way to solve this. Btw: did you run the tests? And would you like to do this together? For this case I have added a new branch dev/escape_entities with a new function get_html_safe_value to replace the call of encode_boolean in preprocess_arguments. It's now up to the new function to return to correct result. For that I have also provided a couple of tests for the new get_html_safe_value function. Now the task is to do the refactoring for running all new tests without breaking anything else. (I have already a solution in mind, but may be we can compare our ways working on this – in case you like to do ;)

chemelli74 commented 2 years ago

Indeed, I love to work in Team. I think is the base of open source projects.

Can we choice a chat platform to allow better interaction?

If I can suggest one, would go for discord.

Simone

kbr commented 2 years ago

Sorry, unexpected emergency weekend – no time for chatting so far. But may be I can push my solution in the next. Just a couple of lines.

chemelli74 commented 2 years ago

Sorry, unexpected emergency weekend – no time for chatting so far. But may be I can push my solution in the next. Just a couple of lines.

No worries; hope all is fine from your side! Yes please push si I can test locally

Simone

kbr commented 2 years ago

Pushed a commit. The single quote has a numerical encoding, in case you like to check.

chemelli74 commented 2 years ago

Pushed a commit. The single quote has a numerical encoding, in case you like to check.

Tested, unfortunately I still get the same error:

fritzconnection.core.exceptions.FritzArgumentError: UPnPError:
errorCode: 402
errorDescription: Invalid Args

Simone

kbr commented 2 years ago

Ok, bad luck. I have made a quick and dirty commit (to dev/escape_entities) without testing the local hardware. Let's hope that the literal entity will work.

chemelli74 commented 2 years ago

Ok, bad luck. I have made a quick and dirty commit (to dev/escape_entities) without testing the local hardware. Let's hope that the literal entity will work.

Sorry to say, the error is still there :-( Could it be there is something else ? With my initial coding based on xml.sax.saxutils.escape() chars was correctly converted [https://github.com/kbr/fritzconnection/issues/111#issuecomment-881900415] but still get the issue. I cannot understand why honestly.

Simone

kbr commented 2 years ago

With both escape methods the conversion is done correctly, sax is just a little outdated. So I also suppose it may be something else. Does the call work without the single quotation mark, like "Lets Encrypt"?

chemelli74 commented 2 years ago

So I also suppose it may be something else. Does the call work without the single quotation mark, like "Lets Encrypt"?

Indeed, if I remove the quote in Fritz UI, everything works fine.

Simone

kbr commented 2 years ago

Ok, in this case it has nothing to do with the other parameters. As the entities are valid we are right back at the beginning: seems that the TR064 interface does not accept the same character-set as the internal UI. May be something to report to AVM. At least now there is entity-encoding ;-)

chemelli74 commented 2 years ago

Ok, in this case it has nothing to do with the other parameters. As the entities are valid we are right back at the beginning: seems that the TR064 interface does not accept the same character-set as the internal UI. May be something to report to AVM. At least now there is entity-encoding ;-)

<s:Body><u:AddPortMapping xmlns:u="urn:dslforum-org:service:WANPPPConnection:1"><NewRemoteHost>0.0.0.0</NewRemoteHost><NewExternalPort>80</NewExternalPort><NewProtocol>TCP</NewProtocol><NewInternalPort>80</NewInternalPort><NewInternalClient>192.168.xxx.yyy</NewInternalClient><NewEnabled>1</NewEnabled><NewPortMappingDescription>Let&apos;s Encrypt Renewal</NewPortMappingDescription><NewLeaseDuration>0</NewLeaseDuration></u:AddPortMapping></s:Body>

I can confirm that the code work fine from python side; I logged a ticket with AVM to check why it doesn't work via TR-064.

I think you can proceed and merge the changes in master. Thank you,

Simone

chemelli74 commented 2 years ago

Code included in ver. 1.6.0