search

fukamachi / fast-http

A fast HTTP request/response parser for Common Lisp.
343 stars 37 forks source link

Reset completedp when keep-alive, parse content length and transfer encoding when store-body is T #4

Closed deadtrickster closed 9 years ago

deadtrickster commented 10 years ago

_Keep-alive and completedp_ completedp should be reset to nil immediately after it returned as T. Or all subsequent parser invocations will invoke finish-callback and of course return T as third value

Response:

 (let* ((response (list #?"HTTP/1.1 200 OK\r\n"
                                #?"Content-Length:  2  \r\n"
                                #?"\r\n"
                                #?"qw"
                                #?"HTTP/1.1 499 This probably should be printed once\r\n"
                                #?"Content-Length:  2  \r\n"
                                #?"\r\n"
                                #?"qw"))
                (http (fast-http:make-http-response))
                (parser (fast-http:make-parser http :finish-callback (lambda () (print http)))))
           (loop for res-chunk in response do              
             (when (nth-value 2 (funcall parser (trivial-utf-8:string-to-utf-8-bytes res-chunk)))
               (print "Message Completed!"))))

#S(FAST-HTTP.HTTP:HTTP-RESPONSE
   :VERSION 11/10
   :HEADERS {

   }
   :STORE-BODY NIL
   :FORCE-STREAM NIL
   :BODY NIL
   :STATUS 200
   :STATUS-TEXT "OK") 
"Message Completed!" 
#S(FAST-HTTP.HTTP:HTTP-RESPONSE
   :VERSION 11/10
   :HEADERS {

   }
   :STORE-BODY NIL
   :FORCE-STREAM NIL
   :BODY NIL
   :STATUS 499
   :STATUS-TEXT "This probably should be printed once") 
"Message Completed!" 
#S(FAST-HTTP.HTTP:HTTP-RESPONSE
   :VERSION 11/10
   :HEADERS {

   }
   :STORE-BODY NIL
   :FORCE-STREAM NIL
   :BODY NIL
   :STATUS 499
   :STATUS-TEXT "This probably should be printed once") 
"Message Completed!" 
#S(FAST-HTTP.HTTP:HTTP-RESPONSE
   :VERSION 11/10
   :HEADERS {

   }
   :STORE-BODY NIL
   :FORCE-STREAM NIL
   :BODY NIL
   :STATUS 499
   :STATUS-TEXT "This probably should be printed once") 
"Message Completed!" 
#S(FAST-HTTP.HTTP:HTTP-RESPONSE
   :VERSION 11/10
   :HEADERS {

   }
   :STORE-BODY NIL
   :FORCE-STREAM NIL
   :BODY NIL
   :STATUS 499
   :STATUS-TEXT "This probably should be printed once") 
"Message Completed!"

Request:

(let* ((request (list #?"POST / HTTP/1.1\r\n"
                              #?"Host: www.example.com\r\n"
                              #?"Content-Type: application/x-www-form-urlencoded\r\n"
                              #?"Content-Length: 4\r\n"
                              #?"\r\n"
                              #?"q=42\r\n"
                              #?"POST / HTTP/1.1\r\n"
                              #?"Host: www.example.com\r\n"
                              #?"Content-Type: application/x-www-form-urlencoded\r\n"
                              #?"Content-Length: 4\r\n"
                              #?"\r\n"
                              #?"q=42\r\n"))
                    (http (fast-http:make-http-request :store-body t ))
                    (parser (fast-http:make-parser http :store-body t )))
               (loop for req-chunk in request do              
                 (when (nth-value 2 (funcall parser (trivial-utf-8:string-to-utf-8-bytes req-chunk)))
                   (print "Message Completed!"))))

"Message Completed!" 
"Message Completed!" 
"Message Completed!" 
"Message Completed!" 
"Message Completed!" 
"Message Completed!" 
"Message Completed!" 
"Message Completed!"

_:store-body t_

While exploring completedp problem I noticed this:

 (let* ((request (list #?"POST / HTTP/1.1\r\n"
                              #?"Host: www.example.com\r\n"
                              #?"Content-Type: application/x-www-form-urlencoded\r\n"
                              #?"Content-Length: 4 \r\n"
                              #?"\r\n"
                              #?"q=42\r\n"))
                    (http (fast-http:make-http-request :store-body t ))
                    (parser (fast-http:make-parser http :store-body t )))
               (loop for req-chunk in request do              
                 (when (nth-value 2 (funcall parser (trivial-utf-8:string-to-utf-8-bytes req-chunk)))
                   (print "Message Completed!"))))

"Message Completed!" 
"Message Completed!"

Here we have just single request, however "Message Completed!" printed twice. Because there no callbacks, headers in highlevel parser were not parsed at all and the following cond clause executed and reset completedp to T:

(T
    ;; No Content-Length, no chunking, probably a request with no body
    (setq completedp t))
fukamachi commented 10 years ago

Though Woo resets the parser by just making a new parser, providing a way to reset it would be better. Values need to reset is not only completedp, but also parser. Low-level parser saves the status which part of HTTP request/response is reading.

fukamachi commented 10 years ago 
(T
    ;; No Content-Length, no chunking, probably a request with no body
    (setq completedp t))

This setq can be omitted, I think.

deadtrickster commented 10 years ago

I'm sorry what is Woo? :-) What if next message starts in the middle of the current chunk(think about HTTP 1.1 pipelining)?

(let* ((response (list #?"HTTP/1.1 200 OK\r\n"
                                #?"Content-Length:  2  \r\n"
                                #?"Content-Type:  application/json  \r\n"
                                #?"\r\n"
                                #?"qwHTTP/1.1 499 second response status?\r\n"
                                #?"Content-Length:  2  \r\n"
                                #?"\r\n"
                                #?"qw"))
                (http (fast-http:make-http-response))
                (parser (fast-http:make-parser http :header-callback (lambda (_)) :finish-callback (lambda () (print http)))))
           (loop for res-chunk in response do              
             (funcall parser (trivial-utf-8:string-to-utf-8-bytes res-chunk))))

#S(FAST-HTTP.HTTP:HTTP-RESPONSE
   :VERSION 11/10
   :HEADERS {
     "content-length" 2
     "content-type" "application/json  "
   }
   :STORE-BODY NIL
   :FORCE-STREAM NIL
   :BODY NIL
   :STATUS 499
   :STATUS-TEXT "second response status?") 
#S(FAST-HTTP.HTTP:HTTP-RESPONSE
   :VERSION 11/10
   :HEADERS {
     "content-length" "2, 2"
     "content-type" "application/json  "
   }
   :STORE-BODY NIL
   :FORCE-STREAM NIL
   :BODY NIL
   :STATUS 499
   :STATUS-TEXT "second response status?")

Also what you think about :store-body part?

fukamachi commented 10 years ago

Woo is a web server I'm working on: https://github.com/fukamachi/woo

What if next message starts in the middle of the current chunk(think about HTTP 1.1 pipelining)?

That's tough. Current fast-http doesn't even call :message-begin callback in this case.

Also what you think about :store-body part?

I'm not sure I could get which part is you're saying, but if it's about this example, deleting (setq completedp t) in cond would work.

 (let* ((request (list #?"POST / HTTP/1.1\r\n"
                              #?"Host: www.example.com\r\n"
                              #?"Content-Type: application/x-www-form-urlencoded\r\n"
                              #?"Content-Length: 4 \r\n"
                              #?"\r\n"
                              #?"q=42\r\n"))
                    (http (fast-http:make-http-request :store-body t ))
                    (parser (fast-http:make-parser http :store-body t )))
               (loop for req-chunk in request do              
                 (when (nth-value 2 (funcall parser (trivial-utf-8:string-to-utf-8-bytes req-chunk)))
                   (print "Message Completed!"))))

"Message Completed!" 
"Message Completed!"
fukamachi commented 9 years ago

I think this is resolved in different way. If there's still a problem, don't hesitate to reopen this.