search

turboladen / playful

A Ruby implementation of UPnP that works with Ruby >= 1.9.x
MIT License
127 stars 18 forks source link

Philips Hue Crashes upnp during a discovery #9

Closed qedi-r closed 10 years ago

qedi-r commented 10 years ago

I am just trying to see if upnp will work for my purposes, and I'm running UPnP::ControlPoint::start (much like the way it happens in the sinatra example...). The issue is that I have a Philips Hue system in my house, and well, it does a lot of things wrong, including UPnP. It sends SSDP notifications with a reference to it's description.xml includes the following serviceList:

<serviceList>
<service>
<serviceType>(null)</serviceType>
<serviceId>(null)</serviceId>
<controlURL>(null)</controlURL>
<eventSubURL>(null)</eventSubURL>
<SCPDURL>(null)</SCPDURL>
</service>
</serviceList>

Of course, then upnp will try to request http://:80/(null) because that is the SCPDURL defined, but it will just return a 404 on the hue and then upnp crashes.

Obviously, that's broken and really the code is doing what it's supposed to, aside from crashing. Ideally, we could have a mechanism to filter out problematic devices from the search as well.

(edit: proper XML quoting)

turboladen commented 10 years ago

Interesting. ...and not too surprising, actually. It seems like there is a lot of variance out there in how people implement UPnP devices/services. I'm a bit of a purist and like the idea of sticking to the spec when dealing with protocols and such, so I'm not too keen on necessarily changing behavior of playful, but I do like the idea of making it easier for users of playful to deal with weird stuff like this.

Without thinking about it a whole lot, I think exception handling could be much improved, and tend to think that this would be the most straightforward way of dealing with this kind of weirdness, but I'm wondering if there may be some other ways to make this more convenient for playful users. Any thoughts/ideas, @qedi-r ?

qedi-r commented 10 years ago

As long as playful doesn't crash, I would agree with not doing too much. I would expect common use cases for finding devices would be to find root nodes on problematic networks and then try and find services and devices afterward, expecting shoddy UPnP implementations like the Philips Hue one here could throw errors.

It looks like the root cause of the crash is that em-http-request actually returns the successful http callback when we get a 404 after trying to request http://<hueip>/(null).xml. I'll have a fix for that, and I will put in the pull request.