CouchDB
  1. CouchDB
  2. COUCHDB-40

Transfer-Encoding: Chunked on HTTP 1.0 request

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.8
    • Component/s: HTTP Interface
    • Labels:
      None
    • Environment:

      Irrelevant

      Description

      The following request:

      > DELETE /test HTTP/1.0
      > Host: localhost

      is responded like:

      > HTTP/1.0 404 Object Not Found
      > Transfer-Encoding: chunked
      > Server: MochiWeb/1.0 (Any of you quaids got a smint?)
      > Date: Tue, 15 Apr 2008 20:39:07 GMT
      > Content-Type: text/plain;charset=utf-8
      >
      > 28
      >

      {"error":"not_found","reason":"missing"}

      > 0

      while chunked transfer-encoding is only supported by HTTP 1.1.

      1. compat_http10_couchdb_r649048.diff
        7 kB
        Benoit Chesneau
      2. mochiweb_chunking_http10_2.diff
        3 kB
        Christopher Lenz
      3. mochiweb_chunking_http10.diff
        2 kB
        Christopher Lenz
      4. patch-share_www_script_jquery_couch_js
        0.9 kB
        Benoit Chesneau
      5. patch-src_couchdb_couch_httpd_erl
        0.7 kB
        Benoit Chesneau

        Activity

        Hide
        Kore Nordmann added a comment -

        This happens since the merge of the mochiweb branch, more specifically in revision 648222.

        Show
        Kore Nordmann added a comment - This happens since the merge of the mochiweb branch, more specifically in revision 648222.
        Hide
        Benoit Chesneau added a comment -

        I can confirm this behaviour. When you use nginx or http proxy wich are http 1.0 proxies, they get chunked http so you can't have proxy over couchdb for now. Tested with this config in nginx :

        server {
        listen 80;
        server_name couchdb;
        location /

        { proxy_pass http://127.0.0.1:5984; proxy_redirect off; proxy_set_header Port $proxy_port; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; }

        }

        nginx display of course unparsed http chunk. For example on index page :

        2e

        {"couchdb":"Welcome","version":"0.7.3a648149"}

        0

        the same with lighttpd :

        $HTTP["host"] == "couchdb"

        { server.document-root = "/var/www/htdocs" proxy.server = ( "" => (( "host" => "127.0.0.1", "port" => 5984 )) ) }
        Show
        Benoit Chesneau added a comment - I can confirm this behaviour. When you use nginx or http proxy wich are http 1.0 proxies, they get chunked http so you can't have proxy over couchdb for now. Tested with this config in nginx : server { listen 80; server_name couchdb; location / { proxy_pass http://127.0.0.1:5984; proxy_redirect off; proxy_set_header Port $proxy_port; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; } } nginx display of course unparsed http chunk. For example on index page : 2e {"couchdb":"Welcome","version":"0.7.3a648149"} 0 the same with lighttpd : $HTTP ["host"] == "couchdb" { server.document-root = "/var/www/htdocs" proxy.server = ( "" => (( "host" => "127.0.0.1", "port" => 5984 )) ) }
        Hide
        Benoit Chesneau added a comment -

        This patch send valid http 1.0 response when you have 1.0 request. I can now post/put/delete doc, remove database. and replicate a database over a proxy. Everything work in HTTP/1.1 with chunks.

        This is a work in progress, for now test suite for unknow reason don't work with such messages :
        1. Exception raised:

        {"message":"syntax error","fileName":"http://couchdb/_utils/script/json2.js","lineNumber":150,"stack":"()@:0\neval(\"()\")@:0\n(\"\")@http://couchdb/_utils/script/json2.js:150\n()@http://couchdb/_utils/script/couch.js:35\n(undefined)@http://couchdb/_utils/script/couch_tests.js:905\nrun(3)@http://couchdb/_utils/script/couch_tests.js:945\n@:0\n","name":"SyntaxError"}

        Firebug show me that DELETE doc is launch 2 times and don't work the second time since the doc don't exist any more. The first delete don't send an ok.

        anyway this patch mostly work, I have to have something stable this week-end.

        Show
        Benoit Chesneau added a comment - This patch send valid http 1.0 response when you have 1.0 request. I can now post/put/delete doc, remove database. and replicate a database over a proxy. Everything work in HTTP/1.1 with chunks. This is a work in progress, for now test suite for unknow reason don't work with such messages : 1. Exception raised: {"message":"syntax error","fileName":"http://couchdb/_utils/script/json2.js","lineNumber":150,"stack":"()@:0\neval(\"()\")@:0\n(\"\")@http://couchdb/_utils/script/json2.js:150\n()@http://couchdb/_utils/script/couch.js:35\n(undefined)@http://couchdb/_utils/script/couch_tests.js:905\nrun(3)@http://couchdb/_utils/script/couch_tests.js:945\n@:0\n","name":"SyntaxError"} Firebug show me that DELETE doc is launch 2 times and don't work the second time since the doc don't exist any more. The first delete don't send an ok. anyway this patch mostly work, I have to have something stable this week-end.
        Hide
        Benoit Chesneau added a comment -

        New patch that fix all issue i had with previous one.

        Now couchdb send valid http 1.0 response (mean no chunks and valid length). Views, attachments, db create/delete, doc create/delete/update work.

        All tests pass. Tested over a nginx proxy.

        ok for that ?

        Show
        Benoit Chesneau added a comment - New patch that fix all issue i had with previous one. Now couchdb send valid http 1.0 response (mean no chunks and valid length). Views, attachments, db create/delete, doc create/delete/update work. All tests pass . Tested over a nginx proxy. ok for that ?
        Hide
        Sebastian Probst Eide added a comment -

        Unfortunately the patch doesn't work on the latest trunk version as of writing this...

        Show
        Sebastian Probst Eide added a comment - Unfortunately the patch doesn't work on the latest trunk version as of writing this...
        Hide
        Benoit Chesneau added a comment -

        mm patch apply well here on latest trunk (r650977) and was reported to work on irc

        patch -p0 -i thepatch in top directory of couchdb sources

        You could also use these patches :
        http://babilu.metavers.net/couchdb/patches/patch-share_www_script_couch_js
        http://babilu.metavers.net/couchdb/patches/patch-src_couchdb_couch_httpd_erl

        Same patch but splitted in two for my openbsd port.

        Show
        Benoit Chesneau added a comment - mm patch apply well here on latest trunk (r650977) and was reported to work on irc patch -p0 -i thepatch in top directory of couchdb sources You could also use these patches : http://babilu.metavers.net/couchdb/patches/patch-share_www_script_couch_js http://babilu.metavers.net/couchdb/patches/patch-src_couchdb_couch_httpd_erl Same patch but splitted in two for my openbsd port.
        Hide
        Sebastian Probst Eide added a comment -

        You are right. It still works on the trunk (r651236).
        share/www/script/couch.js has been replaced with a jquery so if one ignores the first half of the patch and only applies the second half, it works perfectly!

        Thank you very much!

        Show
        Sebastian Probst Eide added a comment - You are right. It still works on the trunk (r651236). share/www/script/couch.js has been replaced with a jquery so if one ignores the first half of the patch and only applies the second half, it works perfectly! Thank you very much!
        Hide
        Benoit Chesneau added a comment - - edited

        afaik couch.js is still used for test suite. It's needed since a body can't be null on http 1.0 which broke all tests before this patch

        Show
        Benoit Chesneau added a comment - - edited afaik couch.js is still used for test suite. It's needed since a body can't be null on http 1.0 which broke all tests before this patch
        Hide
        Sebastian Probst Eide added a comment -

        you are right... I must have been really tired, I couldn't find the file.

        Show
        Sebastian Probst Eide added a comment - you are right... I must have been really tired, I couldn't find the file.
        Hide
        Benoit Chesneau added a comment -

        patch against jquerycouch.js that complete compat_http10_couchdb_r649048.diff . the body shouldn't be null on Post/Put query.

        Show
        Benoit Chesneau added a comment - patch against jquerycouch.js that complete compat_http10_couchdb_r649048.diff . the body shouldn't be null on Post/Put query.
        Hide
        Christopher Lenz added a comment -

        I think it'd be nice if a fix could be done in MochiWeb itself. See this thread:

        http://groups.google.com/group/mochiweb/browse_thread/thread/971a1e70ee9561b9

        I'm attaching a patch for you to try. Please report if this fixes the issue (the JS changes will still be necessary), and I'll file a ticket with the patch against the MochiWeb project.

        Show
        Christopher Lenz added a comment - I think it'd be nice if a fix could be done in MochiWeb itself. See this thread: http://groups.google.com/group/mochiweb/browse_thread/thread/971a1e70ee9561b9 I'm attaching a patch for you to try. Please report if this fixes the issue (the JS changes will still be necessary), and I'll file a ticket with the patch against the MochiWeb project.
        Hide
        Benoit Chesneau added a comment -

        this patch work but only when you add the content-length header to http/1.0 request.
        I don't know yet if it should be handled by mochiweb or not. I think it should but didn't have time yet to extend your patch for this.

        On the other hand a quick patch on couch_httpd.erl allowed me to confirm that. Patch attached.

        Tested on nginx & squid.

        Show
        Benoit Chesneau added a comment - this patch work but only when you add the content-length header to http/1.0 request. I don't know yet if it should be handled by mochiweb or not. I think it should but didn't have time yet to extend your patch for this. On the other hand a quick patch on couch_httpd.erl allowed me to confirm that. Patch attached. Tested on nginx & squid.
        Hide
        Christopher Lenz added a comment -

        Are you saying the response requires a Content-Length header to work with those proxies? The change to MochiWeb does not set the content-length, because that would require buffering in all those instances where we actually use chunking, and that would suck.

        Instead of the content-length, it relies on the server closing the connection when the last piece of data has been sent. I don't see why that should not work with proper HTTP/1.0 clients.

        Show
        Christopher Lenz added a comment - Are you saying the response requires a Content-Length header to work with those proxies? The change to MochiWeb does not set the content-length, because that would require buffering in all those instances where we actually use chunking, and that would suck. Instead of the content-length, it relies on the server closing the connection when the last piece of data has been sent. I don't see why that should not work with proper HTTP/1.0 clients.
        Hide
        Christopher Lenz added a comment -

        The first patch against MochiWeb did not work when the client request included
        a "Connection: Keep-Alive" header. This new version fixes that.

        Show
        Christopher Lenz added a comment - The first patch against MochiWeb did not work when the client request included a "Connection: Keep-Alive" header. This new version fixes that.
        Hide
        Benoit Chesneau added a comment -

        second patch seem to work. Works well with squid but had some oddities with nginx I didn't solve yet.
        On nginx I have to put keepalive_timeout at 0 to have it working. It works with this setting. I suspect some
        problem with keep-alive so.

        However when I add Content-Length header on send_json func, everything works like a charm. I will
        do more tests tomorrow.

        Show
        Benoit Chesneau added a comment - second patch seem to work. Works well with squid but had some oddities with nginx I didn't solve yet. On nginx I have to put keepalive_timeout at 0 to have it working. It works with this setting. I suspect some problem with keep-alive so. However when I add Content-Length header on send_json func, everything works like a charm. I will do more tests tomorrow.
        Hide
        Christopher Lenz added a comment -

        The second patch has been accepted into the MochiWeb project, and has been applied to the version in CouchDB trunk in r652206.

        About adding a Content-Length header... there are many places (for example view results) where we use start_json_response and Resp:write_chunk instead of just send_json; those are the cases where we'd need to do buffering of the complete response if we wanted to set the Content-Length appropriately. So I'm pretty sure that's not a viable option unless we decide on doing response buffering, which I personally don't think would be a good idea.

        Waiting for some more feedback before resolving this. Also, I'd like some details on why those JS changes are necessary. And links, specs, etc?

        Show
        Christopher Lenz added a comment - The second patch has been accepted into the MochiWeb project, and has been applied to the version in CouchDB trunk in r652206. About adding a Content-Length header... there are many places (for example view results) where we use start_json_response and Resp:write_chunk instead of just send_json; those are the cases where we'd need to do buffering of the complete response if we wanted to set the Content-Length appropriately. So I'm pretty sure that's not a viable option unless we decide on doing response buffering, which I personally don't think would be a good idea. Waiting for some more feedback before resolving this. Also, I'd like some details on why those JS changes are necessary. And links, specs, etc?
        Hide
        Benoit Chesneau added a comment -

        Sound goof, did the patch have been tested on all http1/0 proxy ?

        About changes in javascript :

        From the rfc 1945 who define http/1.0 :

        A valid Content-Length is required on all HTTP/1.0 POST requests. An
        HTTP/1.0 server should respond with a 400 (bad request) message if it
        cannot determine the length of the request message's content.

        Proxies like nginx are very strict about it. More over nginx don't accept in proxy mode the POST/PUT request without body like a quick search in google would told you.

        About content-length :

        When an Entity-Body is included with a message, the length of that
        body may be determined in one of two ways. If a Content-Length header
        field is present, its value in bytes represents the length of the
        Entity-Body. Otherwise, the body length is determined by the closing
        of the connection by the server.

        So indeed there is no need to bufferize the response if you close the connection at the end.
        Just better to do it when you can it help the client to optimize the connection. So in the case you could do it like in send_json why not ?

        All this stuff need more tests anyway I already provided some results on ml, did you see them ?

        Show
        Benoit Chesneau added a comment - Sound goof, did the patch have been tested on all http1/0 proxy ? About changes in javascript : From the rfc 1945 who define http/1.0 : A valid Content-Length is required on all HTTP/1.0 POST requests. An HTTP/1.0 server should respond with a 400 (bad request) message if it cannot determine the length of the request message's content. Proxies like nginx are very strict about it. More over nginx don't accept in proxy mode the POST/PUT request without body like a quick search in google would told you. About content-length : When an Entity-Body is included with a message, the length of that body may be determined in one of two ways. If a Content-Length header field is present, its value in bytes represents the length of the Entity-Body. Otherwise, the body length is determined by the closing of the connection by the server. So indeed there is no need to bufferize the response if you close the connection at the end. Just better to do it when you can it help the client to optimize the connection. So in the case you could do it like in send_json why not ? All this stuff need more tests anyway I already provided some results on ml, did you see them ?
        Hide
        Benoit Chesneau added a comment -

        I have some problem with latest change on OpenBSD -current: I can't ask any more to close the connection
        in http headers ("Connecttion: close").
        If I do this I get " Connection reset by peer" error. I don't reproduce it on a 2.6.24 linux kernel though.
        Didn't test it yet with freebsd.

        Show
        Benoit Chesneau added a comment - I have some problem with latest change on OpenBSD -current: I can't ask any more to close the connection in http headers ("Connecttion: close"). If I do this I get " Connection reset by peer" error. I don't reproduce it on a 2.6.24 linux kernel though. Didn't test it yet with freebsd.
        Hide
        Benoit Chesneau added a comment -

        I solve problem of connection reset by peer with a patch against httplib2 :
        http://code.google.com/p/httplib2/issues/detail?id=24 .

        The socket wasn't close if you use connection:close header.

        Show
        Benoit Chesneau added a comment - I solve problem of connection reset by peer with a patch against httplib2 : http://code.google.com/p/httplib2/issues/detail?id=24 . The socket wasn't close if you use connection:close header.
        Hide
        Christopher Lenz added a comment -

        As of r 658408 we only use chunked encoding where it's actually needed. Simple responses use no chunked encoding and include a Content-Length header.

        Show
        Christopher Lenz added a comment - As of r 658408 we only use chunked encoding where it's actually needed. Simple responses use no chunked encoding and include a Content-Length header.
        Hide
        Christopher Lenz added a comment -

        Okay, the Javascript/XHR issues were addressed in r660170, so I think this issue should be fixed.

        Show
        Christopher Lenz added a comment - Okay, the Javascript/XHR issues were addressed in r660170, so I think this issue should be fixed.
        Hide
        Sam Bisbee added a comment -

        Confirmed fix and marked Resolved for a while now.

        Show
        Sam Bisbee added a comment - Confirmed fix and marked Resolved for a while now.

          People

          • Assignee:
            Christopher Lenz
            Reporter:
            Kore Nordmann
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development