bug#13857: Unhandled case in module/web/response.scm

[Daniel Hartwig]

On 3 March 2013 12:55, Jason Earl <address@hidden> wrote:
> On Sat, Mar 02 2013, Daniel Hartwig wrote:
>
>> Hello
>>
>> Which version of guile are you using?  Is it from recent git?  There
>> are perhaps related fixes to the web modules made since January.
>
> Dang it.  I knew that there was stuff that I had forgotten to mention.
> I am using the head of the stable-2.0 branch from git.  I would be happy
> to use the master branch if it makes a difference, but I would have to
> update libgc on the machines that I hack on.  However, the diff between
> the web module in the stable-2.0 branch and master is pretty small.  I
> am pretty confident that it would fail in a similar manner.

Right, using master wouldn't help, but I'm glad to learn that you are
on stable-2.0.

For future reference, when making a report like this it helps to
include the release version or git commit id you are using, at least a
note to say something like “recent stable-2.0”.

> I actually have had this problem since before January, and I switched to
> using guile straight out of git because I was hoping the fixes that Andy
> made would solve my problem.

A prudent first step.

>> On 2 March 2013 15:21, Jason Earl <address@hidden> wrote:
>>>
>>> response.scm does not seem to handle the case where the server does not
>>> specify a content length.  Here's a minimal example that should work,
>>> but doesn't:
>>
>> For non-chunked responses, Content-Length is _almost_ always present.
>>
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> #!/usr/local/bin/guile -s
>>> !#
>>>
>>> (use-modules (srfi srfi-8)
>>>              ((web uri)    #:select (string->uri))
>>>              ((web client) #:select (http-get)))
>>>
>>> (receive (res-headers res-body)
>>>     (http-get (string->uri 
>>> "http://www.blogger.com/feeds/4777343509834060826/posts/default";))
>>>   (display res-body)
>>>   (newline))
>>> --8<---------------cut here---------------end--------------->8---
>>
>> On my testing, this server is using chunked transfer encoding in the
>> response, and your patch should have no effect on that?
>
> Did you actually try using guile as the web client?  I am just asking
> because when I tested using Firefox with live http headers I would get
> different headers than when I used guile.

No.  At the time I was not at a station with recent stable-2.0 ready
to test properly, I only performed a manual check of the headers to
explore possabilities and, having received the chunked
transfer-encoding, presumed the server was always using that.  This
now appears more likely due to the proxy active qon my connection.

It is anyway clear that ‘response-body-port’ is missing the case where
a content-length header is not present and the body is terminated by
the server closing the port.  Some additional care needs to be taken,
e.g. ‘#:keep-alive?’ is incompatible with missing content-length.
Depending on how ‘response-body-port’ is used, I will have to consider
whether to signal an error or do something else in that case.

Thanks for reporting this and hopefully it can be fixed for the next
point release.  As I am currently somewhat involved with the web
modules and related RFCs it is something else I can work on.

>
> Here's the relevant part of the trace I get when using the head of
> stable-2.0:
>
> --8<---------------cut here---------------start------------->8---
> trace: |  |  (read-response-body #<<response> version: (1 . 1) code: 200 
> reason-phrase: …>)
> trace: |  |  |  (response-body-port #<<response> version: (1 . 1) code: 200 
> reason-phra…> …)
> trace: |  |  |  |  (response-transfer-encoding #<<response> version: (1 . 1) 
> code: 200 r…>)
> trace: |  |  |  |  |  (assq transfer-encoding ((p3p . "CP=\"This is not a P3P 
> policy…") …))
> trace: |  |  |  |  |  #f
> trace: |  |  |  |  ()
> trace: |  |  |  |  (member (chunked) ())
> trace: |  |  |  |  #f
> trace: |  |  |  |  (response-content-length #<<response> version: (1 . 1) 
> code: 200 reas…>)
> trace: |  |  |  |  |  (assq content-length ((p3p . "CP=\"This is not a P3P 
> policy! S…") …))
> trace: |  |  |  |  |  #f
> trace: |  |  |  |  #f
> trace: |  |  |  #f
> trace: |  |  |  (and=> #f #<procedure get-bytevector-all (_)>)
> trace: |  |  |  #f
> trace: |  |  |  (eof-object? #f)
> trace: |  |  |  #f
> trace: |  |  #f
> --8<---------------cut here---------------end--------------->8---
>
> And here's the headers that I get when I connect with guile:
>
> --8<---------------cut here---------------start------------->8---
> #<<response> version: (1 . 1) code: 200 reason-phrase: "OK" headers: ((p3p . 
> "CP=\"This is not a P3P policy! See 
> http://www.google.com/support/accounts/bin/answer.py?hl=en&answer=151657 for 
> more info.\"") (content-type application/atom+xml (charset . "UTF-8")) 
> (expires . #<date nanosecond: 0 second: 40 minute: 50 hour: 4 day: 3 month: 3 
> year: 2013 zone-offset: 0>) (date . #<date nanosecond: 0 second: 40 minute: 
> 50 hour: 4 day: 3 month: 3 year: 2013 zone-offset: 0>) (cache-control private 
> (max-age . 0)) (last-modified . #<date nanosecond: 0 second: 43 minute: 58 
> hour: 23 day: 19 month: 2 year: 2013 zone-offset: 0>) (etag 
> "AkQGQnoyfil7ImA9WhBSE0w." . #f) (x-content-type-options . "nosniff") 
> (x-xss-protection . "1; mode=block") (server . "GSE") (connection close)) 
> port: #<closed: file 0>>
> --8<---------------cut here---------------end--------------->8---

Right.  I guess this is when you manually perform the steps of
‘http-get’, correct?  When I do that, I get a content-encoding header
with ‘(chunked)’, but such a header is *not* present when I use
‘http-get’.  I suspect this is due to idiosyncrasies of my connection.

>
>>> Now the reason that I started experimenting with guile in the first
>>> place was that I wanted to learn more about scheme, and fixing this
>>> seemed like a good opportunity at a practical application of my basic
>>> scheme skills.
>>>
>>> So I did a little debugging and created this little patch that fixes
>>> this issue.
>
> [...]
>
>>> Poking at this issue has been quite a bit of fun for me.  So, thanks
>>> for all of your hard work on the system.  Now I must admit that I am
>>> interested in seeing how (and if) this gets fixed.
>>>
>>> Jason
>>>
>>
>> Your undelimited port has only one feature on top of a regular port:
>> handle #:keep-alive?.  Note that this HTTP option is not usable unless
>> the response includes a Content-Length header :-).
>
> Apparently I did not make it clear enough how green I was :).

No, that was fairly clear.  The above comment is a hint that we can
just use ‘(response-port r)’ rather than the undelimited port wrapper,
however some consideration needs to be given to the general usage of
‘response-body-port’ and, in particular, implications of
‘#:keep-alive?’.

Regards