[PATCH]: correctly call build-request

[PATCH]: correctly call build-request

[Alexandru Cojocaru]

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

Hi,
http-post currently sends a GET method instead of POST, I tracked down 
the problem
to extend-request.

Best Regards,
Alexandru Cojocaru

[-- Attachment #2: client.patch --]
[-- Type: text/plain, Size: 1815 bytes --]

From 0d7fab577c878cad2c98d6b257e0a1bec84bc12e Mon Sep 17 00:00:00 2001
From: Alexandru Cojocaru <xojoc@gmx.com>
Date: Tue, 27 Aug 2013 15:59:39 +0200
Subject: [PATCH] fix call to build-request

* module/web/client.scm (extend-request): pass all the parameters to build-request

* module/web/client.scm (sanitize-request): don't allow transfer-coding
  to be set when content-length is 0
---
 module/web/client.scm |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
 mode change 100644 => 100755 module/web/client.scm

diff --git a/module/web/client.scm b/module/web/client.scm
old mode 100644
new mode 100755
index 7d5ea49..e7bccd1
--- a/module/web/client.scm
+++ b/module/web/client.scm
@@ -93,11 +93,14 @@
               (loop (cdr addresses))))))))
 
 (define (extend-request r k v . additional)
-  (let ((r (build-request (request-uri r) #:version (request-version r)
+  (let ((r (build-request (request-uri r)
+                          #:method (request-method r)
+                          #:version (request-version r)
                           #:headers
                           (assoc-set! (copy-tree (request-headers r))
                                       k v)
-                          #:port (request-port r))))
+                          #:port (request-port r)
+                          #:meta (request-meta r))))
     (if (null? additional)
         r
         (apply extend-request r additional))))
@@ -125,7 +128,7 @@ as is the case by default with a request returned by `build-request'."
   (cond
    ((not body)
     (let ((length (request-content-length request)))
-      (if length
+      (when length
           (unless (zero? length)
             (error "content-length, but no body"))
           (when (assq 'transfer-encoding (request-headers request))
-- 
1.7.9

Re: [PATCH]: correctly call build-request

[Ian Price]

Alexandru Cojocaru <xojoc@gmx.com> writes:

> Hi,
> http-post currently sends a GET method instead of POST, I tracked down
> the problem
> to extend-request.
It would have been nice to send an example. It's a little annoying to
work backwards from a patch to a bug. :)

scheme@(guile-user)> (use-modules (web uri) (web client))
scheme@(guile-user)> (let ((out (open-output-string)))
                       (catch #t
                         (lambda ()
                           (http-post (string->uri "http://www.example.com")
                                      #:port out
                                      #:body ""))
                         (lambda _
                           (get-output-string out))))
$15 = "GET / HTTP/1.1\r\nContent-Type: text/plain;charset=utf-8\r\nHost: www.example.com\r\nConnection: close\r\n\r\n"

>  (define (extend-request r k v . additional)
> -  (let ((r (build-request (request-uri r) #:version (request-version r)
> +  (let ((r (build-request (request-uri r)
> +                          #:method (request-method r)
> +                          #:version (request-version r)
>                            #:headers
>                            (assoc-set! (copy-tree (request-headers r))
>                                        k v)
> -                          #:port (request-port r))))
> +                          #:port (request-port r)
> +                          #:meta (request-meta r))))
Seems fine to me.

>      (if (null? additional)
>          r
>          (apply extend-request r additional))))
> @@ -125,7 +128,7 @@ as is the case by default with a request returned by `build-request'."
>    (cond
>     ((not body)
>      (let ((length (request-content-length request)))
> -      (if length
> +      (when length
>            (unless (zero? length)
>              (error "content-length, but no body"))
>            (when (assq 'transfer-encoding (request-headers request))
You've merely switched the case around.

Previously, when length was #f, and transfer-encoding was asked for, you
got an error.

Now, when length is not #f, transfer encoding is asked for, and content
length is zero, you get an error.

The latter does not generalise the former.

The correct thing should be more like (untested)

(when (and length (not (zero? length)))
  (error "content-length, but no body"))
(when (assq 'transfer-encoding (request-headers request))
  (error "transfer-encoding not allowed with no body"))
(values request #vu8)

That said, why do we forbid transfer encoding? Pointless though it may
be, I don't think there is a technical or standards reason why it
shouldn't work.


-- 
Ian Price -- shift-reset.com

"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"

Re: [PATCH]: correctly call build-request

[Alexandru Cojocaru]

On 27.08.2013 17:12, Ian Price wrote:
> It would have been nice to send an example. It's a little annoying to
> work backwards from a patch to a bug. :)
I'll keep that in mind. But I said that the problem was with http-post 
so finding
an example shouldn't have been hard.
> You've merely switched the case around.
Ops.

Best regards,
Alexandru Cojocaru

Re: [PATCH]: correctly call build-request

[Mark H Weaver]

Alexandru Cojocaru <xojoc@gmx.com> writes:
> http-post currently sends a GET method instead of POST, I tracked down
> the problem
> to extend-request.

I pushed the first hunk of your patch to stable-2.0.

    Thanks!
      Mark