Nir Soffer has posted comments on this change.

Change subject: http protocol detection
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/27839/1/vdsm/protocolDetector.py
File vdsm/protocolDetector.py:

Line 196:         # init http handler
Line 197:         pass
Line 198: 
Line 199:     def detect(self, data):
Line 200:         return (data.startswith("POST") and data.startswith("PUT") and
> ?
Sorry for not being clear - your condition will never match anything with 
"and". You need something like this:

    (data.startswith("POST") or data.startswith("PUT") ...) and not "/RPC2" in 
data.

But this will break if /RPC2 is not the path, but included in one of the 
headers of the body of the message. So the above check is wrong in more then 
one way.

If you like to implement this in Python - for xmlrpc (assuming ordering)

    try:
        method, path, version = data.split(" ", 2)
    except ValueError:
        return False
    else:
        return method == "POST" and path in ("/", "/RPC2")

for http:

    try:
        method, rest = data.split(" ", 1)
        return method in ("POST", ...)
    except ValueError:
        return False

The regex way is much simpler and probably faster.
Line 201:                 data.startswith("GET") and data.startswith("HEAD") and
Line 202:                 data.startswith("DELETE") and 
data.startswith("TRANCE") and
Line 203:                 data.startswith("OPTIONS") and not "/RPC2" in data)
Line 204: 


-- 
To view, visit http://gerrit.ovirt.org/27839
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f899cbab4d95ebd184bf32f3ccec1f4fa0e49bc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to