[FEATURE Request] Built-in server should support a thunk as body

[FEATURE Request] Built-in server should support a thunk as body

[Nala Ginrut]

I'm trying to handle static file with our sendfile, but I realized it's
impossible to call it in the handler of run-server.
Although sanitize-response supports procedure as body, it never let me
use sendfile at any chance, because the final writing operation should
be delayed to server-impl-write. If I do it in advanced (in
sanitize-response), the body will appear before the http header, which
is wrong way.

My suggestion is to support thunk as body, the thunk included body
writing operation, and sanitize-response will pass it to the next step
without any cooking. When server-impl-write detected it's a thunk, it'll
call it directly to write the body rather than calling
write-response-body.

Of course, in this way, users have to pass Content-Length manually in
build-response. Consider the size of file will be confirmed when calling
sendfile, it's easy to do that.

In short, my approach is some kind of lazy evaluation for body handling.

I can format a patch if it's agreed.

Comments?

Re: [FEATURE Request] Built-in server should support a thunk as body

[David Thompson]

Hi Nala,

Nala Ginrut <nalaginrut@gmail.com> writes:

> I'm trying to handle static file with our sendfile, but I realized it's
> impossible to call it in the handler of run-server.
> Although sanitize-response supports procedure as body, it never let me
> use sendfile at any chance, because the final writing operation should
> be delayed to server-impl-write. If I do it in advanced (in
> sanitize-response), the body will appear before the http header, which
> is wrong way.
>
> My suggestion is to support thunk as body, the thunk included body
> writing operation, and sanitize-response will pass it to the next step
> without any cooking. When server-impl-write detected it's a thunk, it'll
> call it directly to write the body rather than calling
> write-response-body.
>
> Of course, in this way, users have to pass Content-Length manually in
> build-response. Consider the size of file will be confirmed when calling
> sendfile, it's easy to do that.
>
> In short, my approach is some kind of lazy evaluation for body handling.
>
> I can format a patch if it's agreed.
>
> Comments?

I'm currently writing a web application using Guile's built-in HTTP
server.  To serve static files, I build a response like:

  (values `((content-type . (text/css)))
          (call-with-input-file file-name get-bytevector-all))

Since the response body can be a string, bytevector, or lambda, I tried
using sendfile:

  (values `((content-type . (text/css)))
          (lambda (output)
            (call-with-input-file file-name
              (lambda (input)
                (sendfile output input file-size)))))

However, it didn't work because 'output' is a string port, but sendfile
requires file ports.

Does your proposal give us access to a file port that we can write to?
I'm still learning to use Guile's HTTP modules and serving static files
was something that confused me for awhile.

-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [FEATURE Request] Built-in server should support a thunk as body

[Nala Ginrut]

[-- Attachment #1: Type: text/plain, Size: 2196 bytes --]

hi David!

2014年8月25日 下午8:16于 "David Thompson" <dthompson2@worcester.edu>写道：
>
> Hi Nala,
>
> I'm currently writing a web application using Guile's built-in HTTP
> server.  To serve static files, I build a response like:
>
>   (values `((content-type . (text/css)))
>           (call-with-input-file file-name get-bytevector-all))
>
> Since the response body can be a string, bytevector, or lambda, I tried
> using sendfile:
>
>   (values `((content-type . (text/css)))
>           (lambda (output)
>             (call-with-input-file file-name
>               (lambda (input)
>                 (sendfile output input file-size)))))
>
> However, it didn't work because 'output' is a string port, but sendfile
> requires file ports.
>

You can't use sendfile here because the lambda you passed in will be
expected to output a string.
But we don't need any content string, since the content should be handled
by sendfile automatically.

> Does your proposal give us access to a file port that we can write to?
> I'm still learning to use Guile's HTTP modules and serving static files
> was something that confused me for awhile.
>

My proposal is to pass a thunk, say, a lambda with no arguments.
E.g:
(define (your-func req filename)
  (define in (open-input-file filename))
  (define size (stat:size (stat filename)))
  (define my-response ...)
  (values my-response
                 (lambda ()
                   (sendfile
                     (request-port req)
                     in size))))
This func could be called in handler of run-server.
One have to detect file size with stat and pass it as Content-Length in
#:headers of my-request, I ignored it here, but I think you could
understand.(Sorry but I'm typing these code on my smart phone).

The thunk will be called in http write handler in my patch.

With this feature, we can use sendfile for static file without breaking the
interface.

PS: If you want to build web app quickly, I'll recommend Artanis :-)

> --
> David Thompson
> Web Developer - Free Software Foundation - http://fsf.org
> GPG Key: 0FF1D807
> Support the FSF: https://fsf.org/donate

[-- Attachment #2: Type: text/html, Size: 2874 bytes --]

Re: [FEATURE Request] Built-in server should support a thunk as body

[Nala Ginrut]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

I attached the patch here.
The patch works for me. And I've tested it compared to the old way, say,
read the content to memory then pass it as the body then send to the
client.

Please test this feature with bigger file (larger than 20MB), the small
file is hard to show the difference.

On average, the old way is 5 times slower than new way (10s vs. 50s).

The test code is:
---------------------------code------------------------------
(define (send-the-file r fn)
 (define in (open-input-file fn))
 (define size (stat:size (stat fn)))
 (define res (build-response #:headers `((content-length . ,size))))
 (values res (lambda () (sendfile (request-port r) in size)
                        (close in))))

(run-server
 (lambda (r b)
  (send-the-file r "/var/www/mediawiki-1.21.3.tar.gz"))
 'http)
----------------------------end------------------------------

It's the users duty to pass all the related headers in this way.

The patch was attached.



[-- Attachment #2: 0001-The-built-in-http-server-supports-thunk-as-body-to-t.patch --]
[-- Type: text/x-patch, Size: 1410 bytes --]

From ceac1650327396199dc5114a898233584e4d5d3a Mon Sep 17 00:00:00 2001
From: Nala Ginrut <nalaginrut@gmail.com>
Date: Tue, 26 Aug 2014 13:18:51 +0800
Subject: [PATCH] The built-in http server supports thunk as body to take
 advantage of sendfile

---
 module/web/server.scm      |    6 ++++++
 module/web/server/http.scm |    1 +
 2 files changed, 7 insertions(+)

diff --git a/module/web/server.scm b/module/web/server.scm
index 471bb98..b358fec 100644
--- a/module/web/server.scm
+++ b/module/web/server.scm
@@ -216,6 +216,12 @@ on the procedure being called at any particular time."
            (extend-response response 'content-type
                             `(,@type (charset . ,charset))))
        (string->bytevector body charset))))
+   ((thunk? body)
+    (values
+     response
+     (if (eq? (request-method request) 'HEAD)
+        #f
+        body)))
    ((procedure? body)
     (let* ((type (response-content-type response
                                         '(text/plain)))
diff --git a/module/web/server/http.scm b/module/web/server/http.scm
index cda44f4..3767c61 100644
--- a/module/web/server/http.scm
+++ b/module/web/server/http.scm
@@ -154,6 +154,7 @@
          (port (response-port response)))
     (cond
      ((not body))                       ; pass
+     ((thunk? body) (body))
      ((bytevector? body)
       (write-response-body response body))
      (else
-- 
1.7.10.4

Re: [FEATURE Request] Built-in server should support a thunk as body

[Nala Ginrut]

On Tue, 2014-08-26 at 13:30 +0800, Nala Ginrut wrote:
> I attached the patch here.
> The patch works for me. And I've tested it compared to the old way, say,
> read the content to memory then pass it as the body then send to the
> client.
> 
> Please test this feature with bigger file (larger than 20MB), the small
> file is hard to show the difference.
> 
> On average, the old way is 5 times slower than new way (10s vs. 50s).


Sorry for typo, 0.010s vs. 0.050s