Nmap Development mailing list archives
Re: bugs in http.lua?
From: David Fifield <david () bamsoftware com>
Date: Sat, 12 Dec 2009 19:15:08 -0700
On Sun, Nov 29, 2009 at 11:40:54PM +0100, Patrik Karlsson wrote:
I'm currently re-writinig my Citrix xml plugins to use the http module, but have come a cross some problems. The first problem is that the content I'm sending to the server is of text/xml rather than application/x-www-form-urlencoded. I manage to change this by calling http.post with the following table in the option parameter: { header={["Content-Type"]="text/xml"}} This solves one problem, but the buildPost function then replaces all spaces with pluses, which is probably right for application/x-www-form-urlencoded but in my case it breaks my xml post. I've managed to work around this in my code by calling http.request and http.parseResult directly instead. The downside is that I have to build the http headers myself.
Thanks, Patrik. I don't think the library should be messing with a request body if it is a string. If it's a table then it's a little different because presumably we want to emulate a form submission. I changed the code to only set application/x-www-form-urlencoded and encode the body if it comes in as a table.
The next problem is that the server responds with a "HTTP/1.1 100 Continue". So basically what the http module gets is this: HTTP/1.1 100 Continue Server: Citrix Web PN Server Date: Sun, 29 Nov 2009 22:03:42 GMT HTTP/1.1 200 OK Server: Citrix Web PN Server Date: Sun, 29 Nov 2009 22:03:42 GMT Content-type: text/xml Transfer-Encoding: chunked Transfer-Coding: chunked DATA GOES HERE .... As the module splits the headers from the body by searching for "\r?\n\r?\n" it incorrectly ends up with the second block of HTTP headers as data. By calling http.request directly I have the possibility to cut away this excessive block of headers myself before sending the data along to the parseResult function.
Removing the 100 response is correct. This is what RFC 2616 says: 10.1.1 100 Continue The client SHOULD continue with its request. This interim response is used to inform the client that the initial part of the request has been received and has not yet been rejected by the server. The client SHOULD continue by sending the remainder of the request or, if the request has already been completed, ignore this response. The server MUST send a final response after the request has been completed. See section 8.2.3 for detailed discussion of the use and handling of this status code. However, I don't like the way that the patch does it. -- if we have a 100 Continue, we need to chop it of -- or else the next separation will endup with our 200 OK as body if response:match("^HTTP/1.1 100 Continue") then -- Try to match from HTTP/1.1 200 OK to the end if response:match("(HTTP/1.1 200 OK.+)") then response = response:match("(HTTP/1.1 200 OK.+)") end end The main problem with it is that you're looking for specific values for strings that are allowed to vary. The response might not say "HTTP/1.1" but maybe "HTTP/1.0" or even "HTTP/1.2" in the future. The Reason-Phrases "Continue" and "OK" may be blank, in a different language, or nonsense; the only thing that matters is the status code. Another problem is that the code only allows a 200 status code following the 100. If a POST results in a 301 or a 400 or something else, that's what has to be reported to the caller. See what you can do to handle all these cases. Don't grep for something that looks like a Status-Line ("HTTP/1.1 200 OK") to decide when the next response begins; that way is incorrect even though the http library does it in other places, and it's not necessary here. The 100 response ends after the first "\r?\n\r?\n" because 100 responses aren't allowed to have a body. This is what section 4.3 says: For response messages, whether or not a message-body is included with a message is dependent on both the request method and the response status code (section 6.1.1). All responses to the HEAD request method MUST NOT include a message-body, even though the presence of entity- header fields might lead one to believe they do. All 1xx (informational), 204 (no content), and 304 (not modified) responses MUST NOT include a message-body. All other responses do include a message-body, although it MAY be of zero length. Ideally the process for requests would look like this: status = send_request -- Check for errors. while true do resp = read_response if resp.status % 100 != 100 then break end end return resp But the current design of the http library doesn't work well for that. A "good enough" solution is good enough for now. David Fifield _______________________________________________ Sent through the nmap-dev mailing list http://cgi.insecure.org/mailman/listinfo/nmap-dev Archived at http://seclists.org/nmap-dev/
Current thread:
- bugs in http.lua? Patrik Karlsson (Nov 29)
- Re: bugs in http.lua? Ron (Nov 29)
- Re: bugs in http.lua? Joao Correa (Nov 30)
- Re: bugs in http.lua? Patrik Karlsson (Nov 30)
- Re: bugs in http.lua? Joao Correa (Nov 30)
- Re: bugs in http.lua? David Fifield (Dec 12)
- Re: bugs in http.lua? David Fifield (Dec 12)
- Re: bugs in http.lua? Ron (Nov 29)