GNOME Bugzilla – Bug 696289
should return 412 if NOTIFY does not have proper SID
Last modified: 2013-05-29 13:37:33 UTC
NOTIFY messages should have a SID. SID should start with "uuid:" and should have a unique value after that (The uuid format is also defined but unfortunately only in 1.1). Messages without a proper SID should be replied to with 412 - precondition failed. This is tested by DLNA CTT test 7.2.16.1, and seems like a sane policy to me (as in I don't see how enforcing this could break anything that was working before). Christophe: I don't have a CTT test setup yet, if it's easy for you to test this patch next time you run CTT, that would be great.
Created attachment 239469 [details] [review] Ensure NOTIFY message has a proper SID
Christophe G. checked the patch, it partially fixes it as it allows the test tool to go further with the HTTP error codes testing. Please find below the headers sent by the test tool and the HTTP error codes expected to have the TC successful: SID HDR = (null) -> HTTP 412 SID HDR = uuid: -> HTTP 412 SID HDR = uuid:BADsid -> HTTP 412 NT HDR = (null) -> HTTP 400 NT HDR = upnp:BADevent -> HTTP 412 NTS HDR = (null) -> HTTP 400 NTS HDR = upnp:BADpropchange -> HTTP 412
Isn't that a separate bug? I mean yes, it's the same testcase falling but for different reasons.
(In reply to comment #3) > Isn't that a separate bug? I mean yes, it's the same testcase falling but for > different reasons. I agree. I can file new bugs for any remaining things. (In reply to comment #2) > SID HDR = (null) > -> HTTP 412 > > SID HDR = uuid: > -> HTTP 412 These should now pass. > SID HDR = uuid:BADsid > -> HTTP 412 This can only be handled if we check the version in USER-AGENT first: uuid format was only defined in UPnP/1.1. I guess we could do that. > NT HDR = (null) > -> HTTP 400 > > NTS HDR = (null) > -> HTTP 400 We reply 412 to these currently and spec is indeed clear: Missing NT or NTS should result in a NOTIFY should result in 400. I'll fix this in another patch. > NT HDR = upnp:BADevent > -> HTTP 412 Should work already. > NTS HDR = upnp:BADpropchange > -> HTTP 412 Should work already.
(In reply to comment #4) > > SID HDR = uuid:BADsid > > -> HTTP 412 > > This can only be handled if we check the version in USER-AGENT first: uuid > format was only defined in UPnP/1.1. I guess we could do that. ...now that I checked, NOTIFYs don't have USER-AGENT. Subscription responses do have one (called SERVER to be exact) so we could use that. Unfortunately at least at my place all services advertise "UPnP/1.0" -- even the gupnp based services. > > NT HDR = (null) > > -> HTTP 400 > > > > NTS HDR = (null) > > -> HTTP 400 > > We reply 412 to these currently and spec is indeed clear: Missing NT or NTS > should result in a NOTIFY should result in 400. I'll fix this in another patch. I've filed bug 698192.
(In reply to comment #5) > (In reply to comment #4) > > > SID HDR = uuid:BADsid > > > -> HTTP 412 > > > > This can only be handled if we check the version in USER-AGENT first: uuid > > format was only defined in UPnP/1.1. I guess we could do that. > > ...now that I checked, NOTIFYs don't have USER-AGENT. Subscription responses do > have one (called SERVER to be exact) so we could use that. Unfortunately at > least at my place all services advertise "UPnP/1.0" -- even the gupnp based > services. 1.1 isn't really widespread, unfortunately. GUPnP also only implements 1.0. > > > > NT HDR = (null) > > > -> HTTP 400 > > > > > > NTS HDR = (null) > > > -> HTTP 400 > > > > We reply 412 to these currently and spec is indeed clear: Missing NT or NTS > > should result in a NOTIFY should result in 400. I'll fix this in another patch. > > I've filed bug 698192. Thanks
Attachment 239469 [details] pushed as be0665b - Ensure NOTIFY message has a proper SID