couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Filipe Manana (JIRA)" <j...@apache.org>
Subject [jira] Updated: (COUCHDB-558) Validate Content-MD5 request headers on uploads
Date Sun, 15 Nov 2009 14:42:39 GMT

     [ https://issues.apache.org/jira/browse/COUCHDB-558?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Filipe Manana updated COUCHDB-558:
----------------------------------

    Attachment: jira-couchdb-558-for-trunk-3rd-try.patch

Hi again Paul,

I already improved on some of the points you made:

* check_integrity should probably throw an error or return the body -> DONE
* You should still be recording stats even when validation fails -> DONE
* There are alot of variable assignments where they aren't necessary -> DONE
* keep lines less than 80 characters -> DONE
* really_long_function_names_are_hard_to_read - The functions for trailers could be made more
generic. -> DONE
* The check for Content-MD5 appears to be case sensitive -> DONE
* ...having your hash matching function just throw an error that will get caught by the try/catch
around the HandleReq() call -> DONE

* "the only thing I'm a bit concerned about is the trailier parsing. The current bits are
a bit awkard. In a perfect world id prefer to see that as a patch to mochiweb, but having
it in CouchDB is fine if they rejected that patch or during the time it takes to get into
upstream."

Well, the read_length(0) function from mochiweb_request.erl is awkard, as it gives us the
trailer as a list of binaries. In mochiweb_headers.erl, we can create a Mochiweb Headers structure
only from [ {key(), value()} ] lists.
Therefore, in this patch, I added this little function to couch_httpd.erl:

%% @spec to_mochiweb_headers([binary()]) -> headers()
%%
%% Transforms the given binary list into a Mochiweb
%% headers structure. Each binary is a raw HTTP header
%% line (e.g. <<"Content-Lengh: 345\r\n">>).
%%
to_mochiweb_headers(BinaryList) ->
    {ok, R} = re:compile("^(.*?):\s+(.*?)\r\n$"),
    F = fun(Bin, Acc) ->
        {match, [_, H, V]} = re:run(Bin, R, [{capture, all, list}]),
        [ {H, V} | Acc ]
    end,
    mochiweb_headers:make(lists:foldr(F, [], BinaryList)).

Then I can get values from it like a normal header: "mochiweb_headers:get_value("Content-MD5",
Trailer)".
This is case insensitive.

I would like to know you point of views for:

1) I think I'll submit a patch to Mochiweb, where I add that little function to mochiweb_utils.erl
or mochiweb_headers.erl.

2) Look into the self explanatory comment of the function update_req/2 that I added. What
do you think?

I will write the Erlang test suite soon :)

thanks

Best regards,
Filipe Manana



> Validate Content-MD5 request headers on uploads
> -----------------------------------------------
>
>                 Key: COUCHDB-558
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-558
>             Project: CouchDB
>          Issue Type: Improvement
>          Components: Database Core, HTTP Interface
>            Reporter: Adam Kocoloski
>             Fix For: 0.11
>
>         Attachments: jira-couchdb-558-for-trunk-2nd-try.patch, jira-couchdb-558-for-trunk-3rd-try.patch,
jira-couchdb-558-for-trunk.patch
>
>
> We could detect in-flight data corruption if a client sends a Content-MD5 header along
with the data and Couch validates the MD5 on arrival.
> RFC1864 - The Content-MD5 Header Field
> http://www.faqs.org/rfcs/rfc1864.html

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message