Comments on the web modules

Comments on the web modules

[Josep Portella Florit]

Hi!

I have some comments on the web modules:

1) The web server handlers receive a request object and the body as a
bytevector.  I think it would be more flexible to pass the request
object only, and let the handler decide how to read the body from the
port of the request object; it would allow processing large bodies
without being bound by RAM.

2) The web server handlers must return a response object (or an alist of
headers) and a body, which may be a bytevector, a string or a procedure
that accepts a port argument.  In the case of the procedure, the output
to the port is accumulated into a bytevector, and then written to the
client's port.  I think it should be written to the client's port
directly; it would allow to write large bodies without being bound by
RAM.  This would imply that, if the content-length is not set, the port
should be wrapped by a chunk-encoded port (I know you have already
thought about this, according to the `sanitize-response' documentation),
or else the connection should be closed after the procedure returns.

3) The web server should add the client's sock-addr to the meta alist
of the request object.  I know you planned to do this, according to
the `request-meta' documentation and the (web server http) source.
Instead of taking the `cdr' of `accept's return value and storing it
somewhere, which would complicate the code, you could use
`getpeername' when building the request.  I'm interested in this
because I wrote a procedure that creates request objects from CGI
requests, and I would like to make the meta information compatible.

4) `sanitize-response' adds a default charset parameter to the
response's content-type header when the body is a string or a
procedure, even if the content-type is not textual.  See the code
below.  I think it should only do this for textual content-types
(text/*).

Regards


### `sanitize-response' test

(use-modules (web server)
             (web request)
             (web response)
             (web uri)
             (rnrs io ports))

(define (sanitize-response-test body)
  (call-with-values (lambda ()
                      (sanitize-response
                       (build-request (string->uri "http://test/"))
                       '((content-type . (application/octet-stream)))
                       body))
    (lambda (response body)
      (values (response-headers response)
              body))))


(sanitize-response-test (lambda (port)
                          (put-bytevector port #vu8(1 2 3))))
=> ((content-length . 3)
    (content-type application/octet-stream (charset . "utf-8")))
=> #vu8(1 2 3)


(sanitize-response-test "\x01\x02\x03")
=> ((content-length . 3)
    (content-type application/octet-stream (charset . "utf-8")))
=> #vu8(1 2 3)


(sanitize-response-test #vu8(1 2 3))
=> ((content-length . 3)
    (content-type application/octet-stream))
=> #vu8(1 2 3)

Re: Comments on the web modules

[Nala Ginrut]

Hi Josep!

On Sun, 2014-06-01 at 13:18 +0200, Josep Portella Florit wrote:
> Hi!
> 
> I have some comments on the web modules:
> 
> 1) The web server handlers receive a request object and the body as a
> bytevector.  I think it would be more flexible to pass the request
> object only, and let the handler decide how to read the body from the
> port of the request object; it would allow processing large bodies
> without being bound by RAM.

I agree with you.
The users can't handle the http body by themselves. For an instance, you
have no chance to limit the size of uploaded file, since the http-read
has read all the body bytevectors in the RAM already. And
request-handler in run-server expects http body as the argument.
One of the solution is to add an option to http-read, and return body as
a port when we don't want run-server to read body for us. (Note that #f
couldn't be the proper placeholder since it means 'no body').


[...]

> 3) The web server should add the client's sock-addr to the meta alist
> of the request object.  I know you planned to do this, according to
> the `request-meta' documentation and the (web server http) source.
> Instead of taking the `cdr' of `accept's return value and storing it
> somewhere, which would complicate the code, you could use
> `getpeername' when building the request.  I'm interested in this
> because I wrote a procedure that creates request objects from CGI
> requests, and I would like to make the meta information compatible.
> 

I need sock-addr too.
Besides, I expect there's a way to let user defined server-handler take
the request handler too. 
One of the cases we need it is to support websocket. Once the websocket
handshake is done, users may handle the socket data with client's
sock-addr. And the inner-server don't serve the socket with HTTP
protocol at all.

My request could be hard to compatible with our current server module.
If it's too hard to be in guile-core, I have to write a new server in my
web-framework for full websocket support.

Best regards.