You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by rn...@apache.org on 2023/05/04 16:15:48 UTC

[couchdb] branch remove-content-md5-header created (now 141eb3d19)

This is an automated email from the ASF dual-hosted git repository.

rnewson pushed a change to branch remove-content-md5-header
in repository https://gitbox.apache.org/repos/asf/couchdb.git


      at 141eb3d19 WIP remove Content-MD5 header support

This branch includes the following new commits:

     new 141eb3d19 WIP remove Content-MD5 header support

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[couchdb] 01/01: WIP remove Content-MD5 header support

Posted by rn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rnewson pushed a commit to branch remove-content-md5-header
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 141eb3d19cca2552fff7dc453a65fefebc00b8ce
Author: Robert Newson <rn...@apache.org>
AuthorDate: Thu May 4 10:39:52 2023 +0100

    WIP remove Content-MD5 header support
    
    Part of a series of changes to expunge MD5 entirely.
---
 src/chttpd/src/chttpd_db.erl                       |  34 +-----
 src/couch/src/couch_att.erl                        |  10 +-
 src/couch/src/couch_db.erl                         |  23 +---
 src/couch/src/couch_httpd_db.erl                   |  31 +-----
 src/couch/test/eunit/couchdb_attachments_tests.erl | 122 +--------------------
 test/elixir/test/attachments_test.exs              |  23 ----
 6 files changed, 7 insertions(+), 236 deletions(-)

diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index f2bf447b7..e2de301b2 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -1728,18 +1728,7 @@ db_attachment_req(#httpd{method = 'GET', mochi_req = MochiReq} = Req, Db, DocId,
                             {identity, Ranges} when is_list(Ranges) andalso length(Ranges) < 10 ->
                                 send_ranges_multipart(Req, Type, Len, Att, Ranges);
                             _ ->
-                                Headers1 =
-                                    Headers ++
-                                        if
-                                            Enc =:= identity orelse ReqAcceptsAttEnc =:= true ->
-                                                [
-                                                    {"Content-MD5",
-                                                        base64:encode(couch_att:fetch(md5, Att))}
-                                                ];
-                                            true ->
-                                                []
-                                        end,
-                                {ok, Resp} = start_response_length(Req, 200, Headers1, Len),
+                                {ok, Resp} = start_response_length(Req, 200, Headers, Len),
                                 AttFun(Att, fun(Seg, _) -> send(Resp, Seg) end, {ok, Resp})
                         end
                 end
@@ -1802,7 +1791,6 @@ db_attachment_req(#httpd{method = Method, user_ctx = Ctx} = Req, Db, DocId, File
                         {type, MimeType},
                         {data, Data},
                         {att_len, ContentLen},
-                        {md5, get_md5_header(Req)},
                         {encoding, Encoding}
                     ])
                 ]
@@ -1944,26 +1932,6 @@ parse_ranges([{From, To} | Rest], Len, Acc) ->
 make_content_range(From, To, Len) ->
     ?l2b(io_lib:format("bytes ~B-~B/~B", [From, To, Len])).
 
-get_md5_header(Req) ->
-    ContentMD5 = couch_httpd:header_value(Req, "Content-MD5"),
-    Length = couch_httpd:body_length(Req),
-    Trailer = couch_httpd:header_value(Req, "Trailer"),
-    case {ContentMD5, Length, Trailer} of
-        _ when is_list(ContentMD5) orelse is_binary(ContentMD5) ->
-            base64:decode(ContentMD5);
-        {_, chunked, undefined} ->
-            <<>>;
-        {_, chunked, _} ->
-            case re:run(Trailer, "\\bContent-MD5\\b", [caseless]) of
-                {match, _} ->
-                    md5_in_footer;
-                _ ->
-                    <<>>
-            end;
-        _ ->
-            <<>>
-    end.
-
 parse_doc_query(Req) ->
     lists:foldl(fun parse_doc_query/2, #doc_query_args{}, chttpd:qs(Req)).
 
diff --git a/src/couch/src/couch_att.erl b/src/couch/src/couch_att.erl
index b3b2f23eb..5ca3927e7 100644
--- a/src/couch/src/couch_att.erl
+++ b/src/couch/src/couch_att.erl
@@ -567,14 +567,8 @@ flush_data(Db, Fun, Att) when is_function(Fun) ->
                     % Called with Length == 0 on the last time.
                     % WriterFun returns NewState.
                     fun
-                        ({0, Footers}, _Total) ->
-                            F = mochiweb_headers:from_binary(Footers),
-                            case mochiweb_headers:get_value("Content-MD5", F) of
-                                undefined ->
-                                    ok;
-                                Md5 ->
-                                    {md5, base64:decode(Md5)}
-                            end;
+                        ({0, _Footers}, _Total) ->
+                            ok;
                         ({Length, Chunk}, Total0) ->
                             Total = Total0 + Length,
                             validate_attachment_size(AttName, Total, MaxAttSize),
diff --git a/src/couch/src/couch_db.erl b/src/couch/src/couch_db.erl
index d89d3e3f3..66e9a6db0 100644
--- a/src/couch/src/couch_db.erl
+++ b/src/couch/src/couch_db.erl
@@ -1599,16 +1599,6 @@ compressible_att_type(MimeType) ->
         [T || T <- TypeExpList, T /= []]
     ).
 
-% From RFC 2616 3.6.1 - Chunked Transfer Coding
-%
-%   In other words, the origin server is willing to accept
-%   the possibility that the trailer fields might be silently
-%   discarded along the path to the client.
-%
-% I take this to mean that if "Trailers: Content-MD5\r\n"
-% is present in the request, but there is no Content-MD5
-% trailer, we're free to ignore this inconsistency and
-% pretend that no Content-MD5 exists.
 with_stream(Db, Att, Fun) ->
     [InMd5, Type, Enc] = couch_att:fetch([md5, type, encoding], Att),
     BufferSize = config:get_integer(
@@ -1631,19 +1621,10 @@ with_stream(Db, Att, Fun) ->
                 [{buffer_size, BufferSize}]
         end,
     {ok, OutputStream} = open_write_stream(Db, Options),
-    ReqMd5 =
-        case Fun(OutputStream) of
-            {md5, FooterMd5} ->
-                case InMd5 of
-                    md5_in_footer -> FooterMd5;
-                    _ -> InMd5
-                end;
-            _ ->
-                InMd5
-        end,
+    Fun(OutputStream),
     {StreamEngine, Len, IdentityLen, Md5, IdentityMd5} =
         couch_stream:close(OutputStream),
-    couch_util:check_md5(IdentityMd5, ReqMd5),
+    couch_util:check_md5(IdentityMd5, InMd5),
     {AttLen, DiskLen, NewEnc} =
         case Enc of
             identity ->
diff --git a/src/couch/src/couch_httpd_db.erl b/src/couch/src/couch_httpd_db.erl
index f0786536d..e045510cc 100644
--- a/src/couch/src/couch_httpd_db.erl
+++ b/src/couch/src/couch_httpd_db.erl
@@ -1073,15 +1073,7 @@ db_attachment_req(#httpd{method = 'GET', mochi_req = MochiReq} = Req, Db, DocId,
                                 ->
                                     send_ranges_multipart(Req, Type, Len, Att, Ranges);
                                 _ ->
-                                    Headers1 =
-                                        Headers ++
-                                            if
-                                                Enc =:= identity orelse ReqAcceptsAttEnc =:= true ->
-                                                    [{"Content-MD5", base64:encode(Md5)}];
-                                                true ->
-                                                    []
-                                            end,
-                                    {ok, Resp} = start_response_length(Req, 200, Headers1, Len),
+                                    {ok, Resp} = start_response_length(Req, 200, Headers, Len),
                                     AttFun(Att, fun(Seg, _) -> send(Resp, Seg) end, {ok, Resp})
                             end
                     end
@@ -1174,7 +1166,6 @@ db_attachment_req(
                         {type, MimeType},
                         {data, Data},
                         {att_len, AttLen},
-                        {md5, get_md5_header(Req)},
                         {encoding, Encoding}
                     ])
                 ]
@@ -1250,26 +1241,6 @@ parse_ranges([{From, none} | Rest], Len, Acc) ->
 parse_ranges([{From, To} | Rest], Len, Acc) ->
     parse_ranges(Rest, Len, [{From, To}] ++ Acc).
 
-get_md5_header(Req) ->
-    ContentMD5 = couch_httpd:header_value(Req, "Content-MD5"),
-    Length = couch_httpd:body_length(Req),
-    Trailer = couch_httpd:header_value(Req, "Trailer"),
-    case {ContentMD5, Length, Trailer} of
-        _ when is_list(ContentMD5) orelse is_binary(ContentMD5) ->
-            base64:decode(ContentMD5);
-        {_, chunked, undefined} ->
-            <<>>;
-        {_, chunked, _} ->
-            case re:run(Trailer, "\\bContent-MD5\\b", [caseless]) of
-                {match, _} ->
-                    md5_in_footer;
-                _ ->
-                    <<>>
-            end;
-        _ ->
-            <<>>
-    end.
-
 parse_doc_query(Req) ->
     lists:foldl(
         fun({Key, Value}, Args) ->
diff --git a/src/couch/test/eunit/couchdb_attachments_tests.erl b/src/couch/test/eunit/couchdb_attachments_tests.erl
index 376553985..ef42b9f15 100644
--- a/src/couch/test/eunit/couchdb_attachments_tests.erl
+++ b/src/couch/test/eunit/couchdb_attachments_tests.erl
@@ -102,13 +102,7 @@ attachments_md5_tests() ->
             fun teardown/1,
             [
                 fun should_upload_attachment_without_md5/1,
-                fun should_upload_attachment_by_chunks_without_md5/1,
-                fun should_upload_attachment_with_valid_md5_header/1,
-                fun should_upload_attachment_by_chunks_with_valid_md5_header/1,
-                fun should_upload_attachment_by_chunks_with_valid_md5_trailer/1,
-                fun should_reject_attachment_with_invalid_md5/1,
-                fun should_reject_chunked_attachment_with_invalid_md5/1,
-                fun should_reject_chunked_attachment_with_invalid_md5_trailer/1
+                fun should_upload_attachment_by_chunks_without_md5/1
             ]
         }
     }.
@@ -207,120 +201,6 @@ should_upload_attachment_by_chunks_without_md5({Host, DbName}) ->
         ?assertEqual(true, get_json(Json, [<<"ok">>]))
     end).
 
-should_upload_attachment_with_valid_md5_header({Host, DbName}) ->
-    ?_test(begin
-        AttUrl = string:join(["", DbName, ?docid(), "readme.txt"], "/"),
-        Body = "We all live in a yellow submarine!",
-        Headers = [
-            {"Content-Length", "34"},
-            {"Content-Type", "text/plain"},
-            {"Content-MD5", ?b2l(base64:encode(couch_hash:md5_hash(Body)))},
-            {"Host", Host}
-        ],
-        {ok, Code, Json} = request("PUT", AttUrl, Headers, Body),
-        ?assertEqual(201, Code),
-        ?assertEqual(true, get_json(Json, [<<"ok">>]))
-    end).
-
-should_upload_attachment_by_chunks_with_valid_md5_header({Host, DbName}) ->
-    ?_test(begin
-        AttUrl = string:join(["", DbName, ?docid(), "readme.txt"], "/"),
-        AttData = <<"We all live in a yellow submarine!">>,
-        <<Part1:21/binary, Part2:13/binary>> = AttData,
-        Body = [chunked_body([Part1, Part2]), "\r\n"],
-        Headers = [
-            {"Content-Type", "text/plain"},
-            {"Content-MD5", ?b2l(base64:encode(couch_hash:md5_hash(AttData)))},
-            {"Host", Host},
-            {"Transfer-Encoding", "chunked"}
-        ],
-        {ok, Code, Json} = request("PUT", AttUrl, Headers, Body),
-        ?assertEqual(201, Code),
-        ?assertEqual(true, get_json(Json, [<<"ok">>]))
-    end).
-
-should_upload_attachment_by_chunks_with_valid_md5_trailer({Host, DbName}) ->
-    ?_test(begin
-        AttUrl = string:join(["", DbName, ?docid(), "readme.txt"], "/"),
-        AttData = <<"We all live in a yellow submarine!">>,
-        <<Part1:21/binary, Part2:13/binary>> = AttData,
-        Body = [
-            chunked_body([Part1, Part2]),
-            "Content-MD5: ",
-            base64:encode(couch_hash:md5_hash(AttData)),
-            "\r\n\r\n"
-        ],
-        Headers = [
-            {"Content-Type", "text/plain"},
-            {"Host", Host},
-            {"Trailer", "Content-MD5"},
-            {"Transfer-Encoding", "chunked"}
-        ],
-        {ok, Code, Json} = request("PUT", AttUrl, Headers, Body),
-        ?assertEqual(201, Code),
-        ?assertEqual(true, get_json(Json, [<<"ok">>]))
-    end).
-
-should_reject_attachment_with_invalid_md5({Host, DbName}) ->
-    ?_test(begin
-        AttUrl = string:join(["", DbName, ?docid(), "readme.txt"], "/"),
-        Body = "We all live in a yellow submarine!",
-        Headers = [
-            {"Content-Length", "34"},
-            {"Content-Type", "text/plain"},
-            {"Content-MD5", ?b2l(base64:encode(<<"foobar!">>))},
-            {"Host", Host}
-        ],
-        {ok, Code, Json} = request("PUT", AttUrl, Headers, Body),
-        ?assertEqual(400, Code),
-        ?assertEqual(
-            <<"content_md5_mismatch">>,
-            get_json(Json, [<<"error">>])
-        )
-    end).
-
-should_reject_chunked_attachment_with_invalid_md5({Host, DbName}) ->
-    ?_test(begin
-        AttUrl = string:join(["", DbName, ?docid(), "readme.txt"], "/"),
-        AttData = <<"We all live in a yellow submarine!">>,
-        <<Part1:21/binary, Part2:13/binary>> = AttData,
-        Body = [chunked_body([Part1, Part2]), "\r\n"],
-        Headers = [
-            {"Content-Type", "text/plain"},
-            {"Content-MD5", ?b2l(base64:encode(<<"foobar!">>))},
-            {"Host", Host},
-            {"Transfer-Encoding", "chunked"}
-        ],
-        {ok, Code, Json} = request("PUT", AttUrl, Headers, Body),
-        ?assertEqual(400, Code),
-        ?assertEqual(
-            <<"content_md5_mismatch">>,
-            get_json(Json, [<<"error">>])
-        )
-    end).
-
-should_reject_chunked_attachment_with_invalid_md5_trailer({Host, DbName}) ->
-    ?_test(begin
-        AttUrl = string:join(["", DbName, ?docid(), "readme.txt"], "/"),
-        AttData = <<"We all live in a yellow submarine!">>,
-        <<Part1:21/binary, Part2:13/binary>> = AttData,
-        Body = [
-            chunked_body([Part1, Part2]),
-            "Content-MD5: ",
-            base64:encode(<<"foobar!">>),
-            "\r\n\r\n"
-        ],
-        Headers = [
-            {"Content-Type", "text/plain"},
-            {"Host", Host},
-            {"Trailer", "Content-MD5"},
-            {"Transfer-Encoding", "chunked"}
-        ],
-        {ok, Code, Json} = request("PUT", AttUrl, Headers, Body),
-        ?assertEqual(400, Code),
-        ?assertEqual(<<"content_md5_mismatch">>, get_json(Json, [<<"error">>]))
-    end).
-
 should_get_att_without_accept_gzip_encoding(_, {Data, {_, _, AttUrl}}) ->
     ?_test(begin
         {ok, Code, Headers, Body} = test_request:get(AttUrl),
diff --git a/test/elixir/test/attachments_test.exs b/test/elixir/test/attachments_test.exs
index f1dd3ef61..13a5c84f9 100644
--- a/test/elixir/test/attachments_test.exs
+++ b/test/elixir/test/attachments_test.exs
@@ -430,29 +430,6 @@ defmodule AttachmentsTest do
     assert resp.body["error"] == "missing_stub"
   end
 
-  @tag :with_db
-  test "md5 header for attachments", context do
-    db_name = context[:db_name]
-    md5 = "MntvB0NYESObxH4VRDUycw=="
-
-    bin_data = "foo bar"
-
-    resp =
-      Couch.put(
-        "/#{db_name}/bin_doc8/attachment.txt",
-        body: bin_data,
-        headers: ["Content-Type": "application/octet-stream", "Content-MD5": md5],
-        query: %{w: 3}
-      )
-
-    assert resp.status_code in [201, 202]
-    assert resp.body["ok"]
-
-    resp = Couch.get("/#{db_name}/bin_doc8/attachment.txt")
-    assert resp.status_code == 200
-    assert md5 == resp.headers["Content-MD5"]
-  end
-
   @tag :with_db
   test "attachment via multipart/form-data", context do
     db_name = context[:db_name]