MerkleIntegritySourceStream: Update the treatment of empty payload

This makes MerkleIntegritySourceStream match the new behavior of
draft-thomson-http-mice-03 [1], where the encoding of an empty payload
does not have the 8-byte record size.

Also, after this patch the final record must not be empty. Empty final
records were allowed because http-mice-02 couldn't represent the empty
payload otherwise.

[1] https://tools.ietf.org/html/draft-thomson-http-mice-03#section-2

Bug: 875721
Change-Id: I2e2003603fb9f3d9baf4cf369cf9487b8990bcf8
Reviewed-on: https://chromium-review.googlesource.com/1183044
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585383}

content/browser/loader/merkle_integrity_source_stream.cc

@@ -89,7 +89,17 @@ bool MerkleIntegritySourceStream::FilterDataImpl(base::span<char>* output,
   if (record_size_ == 0) {
     base::span<const char> bytes;
     if (!ConsumeBytes(input, 8, &bytes, &storage)) {
-      return !upstream_eof_reached;
+      if (!upstream_eof_reached) {
+        return true;  // Wait for more data later.
+      }
+      if (partial_input_.empty()) {
+        // As a special case, the encoding of an empty payload is itself an
+        // empty message (i.e. it omits the initial record size), and its
+        // integrity proof is SHA-256("\0").
+        final_record_done_ = true;
+        return ProcessRecord({}, final_record_done_, output);
+      }
+      return false;
     }
     uint64_t record_size;
     base::ReadBigEndian(bytes.data(), &record_size);
@@ -122,14 +132,8 @@ bool MerkleIntegritySourceStream::FilterDataImpl(base::span<char>* output,
       }
 
       // The final record is shorter and does not contain a hash. Process all
-      // remaining input the final record.
-      //
-      // TODO(davidben): This matches the previous implementation in that it
-      // allows empty final records, but this does not match the specification
-      // and means some inputs have two valid encodings. However, the
-      // specification's version cannot represent the empty string. Update this
-      // when https://github.com/martinthomson/http-mice/issues/3 is resolved.
-      if (partial_input_.size() > record_size_) {
+      // remaining input as the final record.
+      if (partial_input_.empty() || partial_input_.size() > record_size_) {
         return false;
       }
       record = partial_input_;

content/browser/loader/merkle_integrity_source_stream_unittest.cc

@@ -159,6 +159,14 @@ TEST_P(MerkleIntegritySourceStreamTest, EmptyStream) {
   source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode);
   std::string actual_output;
   int result = ReadStream(&actual_output);
+  EXPECT_EQ(net::OK, result);
+}
+
+TEST_P(MerkleIntegritySourceStreamTest, EmptyStreamWrongHash) {
+  Init(kMISingleRecord);
+  source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode);
+  std::string actual_output;
+  int result = ReadStream(&actual_output);
   EXPECT_EQ(net::ERR_CONTENT_DECODING_FAILED, result);
 }
 
@@ -210,7 +218,7 @@ TEST_P(MerkleIntegritySourceStreamTest, RecordSizeOnly) {
   source()->AddReadResult(nullptr, 0, net::OK, GetParam().mode);
   std::string actual_output;
   int result = ReadStream(&actual_output);
-  EXPECT_EQ(net::OK, result);
+  EXPECT_EQ(net::ERR_CONTENT_DECODING_FAILED, result);
 }
 
 TEST_P(MerkleIntegritySourceStreamTest, TruncatedRecordSize) {
@@ -462,12 +470,7 @@ TEST_P(MerkleIntegritySourceStreamTest, Truncated) {
   EXPECT_EQ(kMessage, actual_output);
 }
 
-// Test that the final record is allowed to be empty.
-//
-// TODO(davidben): This does not match the specification and means some inputs
-// have two valid encodings. However, the specification's version cannot
-// represent the empty string. Update the code and possibly this test depending
-// on how https://github.com/martinthomson/http-mice/issues/3 is resolved.
+// Test that the final record is not allowed to be empty.
 TEST_P(MerkleIntegritySourceStreamTest, EmptyFinalRecord) {
   Init("mi-sha256-03=JJnIuaOEc2247K9V88VQAQy1GJuQ6ylaVM7mG69QkE4=");
   const uint8_t kRecordSize[] = {0, 0, 0, 0, 0, 0, 0, 16};
@@ -492,7 +495,7 @@ TEST_P(MerkleIntegritySourceStreamTest, EmptyFinalRecord) {
 
   std::string actual_output;
   int rv = ReadStream(&actual_output);
-  EXPECT_EQ(static_cast<int>(kMessage.size()), rv);
+  EXPECT_EQ(net::ERR_CONTENT_DECODING_FAILED, rv);
   EXPECT_EQ(kMessage, actual_output);
 }