christgau / wsdd

A Web Service Discovery host daemon.
MIT License
860 stars 101 forks source link

fix(src): Handle multiple xaddrs in ResolveMatch/ProbeMatch #174

Closed ondrejholy closed 1 year ago

ondrejholy commented 1 year ago

Currently, the ProbeMatch/ResolveMatch messages with multiple transport addresses cause the http.client.InvalidURL: URL can't contain control characters. exceptions. Those messages seem to be valid and can be received e.g. from HP printers. Let's use the same approach as it was implemented for the Hello message over the https://github.com/christgau/wsdd/commit/918a1ae8a3e600366ccdf936bb247a43143f298f commit to fix this issue.

Related: https://github.com/christgau/wsdd/issues/89 Fixes: https://github.com/christgau/wsdd/issues/149

ondrejholy commented 1 year ago

Just a note that my HP printer returns two IPv4 addresses, but only one of them is accessible. Perhaps wsdd should not choose the first one, but the one from which the message was received. Or alternatively, use all of them... what do you think?

ondrejholy commented 1 year ago

I've just tested the updated patch and works fine as well.

christgau commented 1 year ago

Just a note that my HP printer returns two IPv4 addresses, but only one of them is accessible.

Just out of curiosity: Is it because of the printer emitting an invalid address (i.e., an address that is not assigned to the device) or is it an address that you cannot reach because of some firewall issue?

Perhaps wsdd should not choose the first one, but the one from which the message was received. Or alternatively, use all of them... what do you think?

You're right. That would be a more robust way to discover devices and maybe more in line with the standard (I haven't looked it up). But it involves more than just iterating over all xaddrs since the code for the meta data exchange is unfortunately not written in an asynchronous way at the moment. Some refactoring in WSDClient.perform_metadata_exchange would be required first to address the issue appropriately.

ondrejholy commented 1 year ago

Just out of curiosity: Is it because of the printer emitting an invalid address (i.e., an address that is not assigned to the device) or is it an address that you cannot reach because of some firewall issue?

The inaccessible address is for Wi-Fi Direct connection that is enabled on the printer, but I am not connected to it.

christgau commented 1 year ago

The inaccessible address is for Wi-Fi Direct connection that is enabled on the printer, but I am not connected to it.

Ok, thanks. I understand.

baybal commented 1 year ago

Just a note that my HP printer returns two IPv4 addresses, but only one of them is accessible. Perhaps wsdd should not choose the first one, but the one from which the message was received. Or alternatively, use all of them... what do you think?

Another case: one address being IPV4 and another IPV6. Canon MFDs advertise on both V6 and V4. Windows 10 recognises that, and only shows V6.

christgau commented 1 year ago

Ok, I see. But this will require some more code work to get it running