Spanify GzipHeader.

Also make it take uint8_ts rather than chars, since it's dealing with
binary data.

Additionally, introduce a HasGZipHeader() helper, which is what all
but one of its consumers actually want, and migrate consumers to the
new helper.

Bug: 40284755
Change-Id: Icb36a225752e8c56eace06f6e1e4d46c95e45339
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6253892
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: mmenke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1421987}

chromeos/printing/ppd_line_reader.cc

@@ -13,6 +13,7 @@
 #include <string>
 #include <utility>
 
+#include "base/containers/span.h"
 #include "base/functional/bind.h"
 #include "base/memory/raw_ref.h"
 #include "base/strings/string_util.h"
@@ -27,14 +28,6 @@ namespace {
 
 constexpr char kPPDMagicNumberString[] = "*PPD-Adobe:";
 
-// Return true if contents has a valid Gzip header.
-bool IsGZipped(const std::string& contents) {
-  const char* unused;
-  return net::GZipHeader().ReadMore(contents.data(), contents.size(),
-                                    &unused) ==
-         net::GZipHeader::COMPLETE_HEADER;
-}
-
 // Return true if c is a newline in the ppd sense, that is, either newline or
 // carriage return.
 bool IsNewline(char c) {
@@ -78,7 +71,7 @@ class PpdLineReaderImpl : public PpdLineReader {
         read_buf_(
             base::MakeRefCounted<net::IOBufferWithSize>(kReadBufCapacity)) {
     input_ = std::make_unique<StringSourceStream>(ppd_contents);
-    if (IsGZipped(ppd_contents)) {
+    if (net::GZipHeader::HasGZipHeader(base::as_byte_span(ppd_contents))) {
       input_ = net::GzipSourceStream::Create(std::move(input_),
                                              net::SourceStream::TYPE_GZIP);
     }

content/public/test/browser_test_utils.cc

@@ -428,17 +428,6 @@ class TestNavigationManagerThrottle : public NavigationThrottle {
 };
 
 #if BUILDFLAG(IS_CHROMEOS)
-bool HasGzipHeader(const base::RefCountedMemory& maybe_gzipped) {
-  net::GZipHeader header;
-  net::GZipHeader::Status header_status = net::GZipHeader::INCOMPLETE_HEADER;
-  const char* header_end = nullptr;
-  while (header_status == net::GZipHeader::INCOMPLETE_HEADER) {
-    auto chars = base::as_chars(base::span(maybe_gzipped));
-    header_status = header.ReadMore(chars.data(), chars.size(), &header_end);
-  }
-  return header_status == net::GZipHeader::COMPLETE_HEADER;
-}
-
 void AppendGzippedResource(const base::RefCountedMemory& encoded,
                            std::string* to_append) {
   auto source_stream = std::make_unique<net::MockSourceStream>();
@@ -2021,7 +2010,7 @@ bool ExecuteWebUIResourceTest(WebContents* web_contents) {
       ui::ResourceBundle::GetSharedInstance().LoadDataResourceBytes(
           IDR_ASH_WEBUI_COMMON_WEBUI_RESOURCE_TEST_JS);
 
-  if (HasGzipHeader(*bytes)) {
+  if (net::GZipHeader::HasGZipHeader(base::span(*bytes))) {
     AppendGzippedResource(*bytes, &script);
   } else {
     auto chars = base::as_chars(base::span(*bytes));

net/filter/gzip_header.cc

@@ -2,23 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifdef UNSAFE_BUFFERS_BUILD
-// TODO(crbug.com/40284755): Remove this and spanify to fix the errors.
-#pragma allow_unsafe_buffers
-#endif
-
 #include "net/filter/gzip_header.h"
 
+#include <stdint.h>
 #include <string.h>
 
 #include <algorithm>
+#include <array>
 
 #include "base/check_op.h"
+#include "base/containers/span.h"
 #include "third_party/zlib/zlib.h"
 
 namespace net {
 
-const uint8_t GZipHeader::magic[] = {0x1f, 0x8b};
+// gzip magic header.
+static constexpr std::array<uint8_t, 2> kGzipHeaderBytes = {0x1f, 0x8b};
 
 GZipHeader::GZipHeader() {
   Reset();
@@ -32,21 +31,23 @@ void GZipHeader::Reset() {
   extra_length_ = 0;
 }
 
-GZipHeader::Status GZipHeader::ReadMore(const char* inbuf,
-                                        size_t inbuf_len,
-                                        const char** header_end) {
-  const uint8_t* pos = reinterpret_cast<const uint8_t*>(inbuf);
-  const uint8_t* const end = pos + inbuf_len;
+GZipHeader::Status GZipHeader::ReadMore(base::span<const uint8_t> inbuf,
+                                        size_t& header_end) {
+  auto pos = inbuf.begin();
 
-  while ( pos < end ) {
+  while (pos != inbuf.end()) {
     switch ( state_ ) {
       case IN_HEADER_ID1:
-        if ( *pos != magic[0] )  return INVALID_HEADER;
+        if (*pos != kGzipHeaderBytes[0]) {
+          return INVALID_HEADER;
+        }
         pos++;
         state_++;
         break;
       case IN_HEADER_ID2:
-        if ( *pos != magic[1] )  return INVALID_HEADER;
+        if (*pos != kGzipHeaderBytes[1]) {
+          return INVALID_HEADER;
+        }
         pos++;
         state_++;
         break;
@@ -112,8 +113,8 @@ GZipHeader::Status GZipHeader::ReadMore(const char* inbuf,
       case IN_FEXTRA: {
         // Grab the rest of the bytes in the extra field, or as many
         // of them as are actually present so far.
-        const uint16_t num_extra_bytes = static_cast<uint16_t>(
-            std::min(static_cast<ptrdiff_t>(extra_length_), (end - pos)));
+        const uint16_t num_extra_bytes = static_cast<uint16_t>(std::min(
+            static_cast<ptrdiff_t>(extra_length_), (inbuf.end() - pos)));
         pos += num_extra_bytes;
         extra_length_ -= num_extra_bytes;
         if ( extra_length_ == 0 ) {
@@ -128,15 +129,18 @@ GZipHeader::Status GZipHeader::ReadMore(const char* inbuf,
           state_ = IN_FCOMMENT;
           break;
         }
-        // See if we can find the end of the \0-terminated FNAME field.
-        pos = reinterpret_cast<const uint8_t*>(memchr(pos, '\0', (end - pos)));
-        if (pos != nullptr) {
-          pos++;  // advance past the '\0'
-          flags_ &= ~FLAG_FNAME;   // we're done with the FNAME stuff
+        // See if we can find the end of the null-byte-terminated FNAME field.
+        pos = std::find(pos, inbuf.end(), 0u);
+        // If the null was found, the end of the FNAME has been reached, and
+        // need to advance to the next character.
+        if (pos != inbuf.end()) {
+          pos++;                  // Advance past the null-byte.
+          flags_ &= ~FLAG_FNAME;  // We're done with the FNAME stuff.
           state_ = IN_FCOMMENT;
-        } else {
-          pos = end;  // everything we have so far is part of the FNAME
         }
+        // Otherwise, everything so far is part of the FNAME, and still have to
+        // continue looking for the null byte, so nothing else to do until more
+        // data is received.
         break;
 
       case IN_FCOMMENT:
@@ -144,15 +148,19 @@ GZipHeader::Status GZipHeader::ReadMore(const char* inbuf,
           state_ = IN_FHCRC_BYTE_0;
           break;
         }
-        // See if we can find the end of the \0-terminated FCOMMENT field.
-        pos = reinterpret_cast<const uint8_t*>(memchr(pos, '\0', (end - pos)));
-        if (pos != nullptr) {
-          pos++;  // advance past the '\0'
-          flags_ &= ~FLAG_FCOMMENT;   // we're done with the FCOMMENT stuff
+        // See if we can find the end of the null-byte-terminated FCOMMENT
+        // field.
+        pos = std::find(pos, inbuf.end(), 0u);
+        // If the null was found, the end of the FNAME has been reached, and
+        // need to advance to the next character.
+        if (pos != inbuf.end()) {
+          pos++;                     // Advance past the null-byte.
+          flags_ &= ~FLAG_FCOMMENT;  // We're done with the FCOMMENT stuff.
           state_ = IN_FHCRC_BYTE_0;
-        } else {
-          pos = end;  // everything we have so far is part of the FNAME
         }
+        // Otherwise, everything so far is part of the FCOMMENT, and still have
+        // to continue looking for the null byte, so nothing else to do until
+        // more data is received.
         break;
 
       case IN_FHCRC_BYTE_0:
@@ -171,17 +179,22 @@ GZipHeader::Status GZipHeader::ReadMore(const char* inbuf,
         break;
 
       case IN_DONE:
-        *header_end = reinterpret_cast<const char*>(pos);
+        header_end = pos - inbuf.begin();
         return COMPLETE_HEADER;
     }
   }
 
   if ( (state_ > IN_HEADER_OS) && (flags_ == 0) ) {
-    *header_end = reinterpret_cast<const char*>(pos);
+    header_end = pos - inbuf.begin();
     return COMPLETE_HEADER;
   } else {
     return INCOMPLETE_HEADER;
   }
 }
 
+bool GZipHeader::HasGZipHeader(base::span<const uint8_t> inbuf) {
+  size_t ignored_header_end = 0u;
+  return GZipHeader().ReadMore(inbuf, ignored_header_end) == COMPLETE_HEADER;
+}
+
 }  // namespace net

net/filter/gzip_header.h

@@ -18,6 +18,7 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include "base/containers/span.h"
 #include "net/base/net_export.h"
 
 namespace net {
@@ -45,9 +46,14 @@ class NET_EXPORT GZipHeader {
   // yet constitute a complete gzip header, return
   // INCOMPLETE_HEADER. If these bytes do not constitute a *valid*
   // gzip header, return INVALID_HEADER. When we've seen a complete
-  // gzip header, return COMPLETE_HEADER and set the pointer pointed
-  // to by header_end to the first byte beyond the gzip header.
-  Status ReadMore(const char* inbuf, size_t inbuf_len, const char** header_end);
+  // gzip header, return COMPLETE_HEADER and set `header_end` to the offset
+  // of the first byte beyond the gzip header (i.e., it's the number of bytes of
+  // `inbuf` the are part of the header).
+  Status ReadMore(base::span<const uint8_t> inbuf, size_t& header_end);
+
+  // Returns true if `inbuf` starts with a gzip header, possibly followed by
+  // additional bytes.
+  static bool HasGZipHeader(base::span<const uint8_t> inbuf);
 
  private:
   enum {                       // flags (see RFC)
@@ -86,8 +92,6 @@ class NET_EXPORT GZipHeader {
     IN_DONE,
   };
 
-  static const uint8_t magic[];  // gzip magic header
-
   int    state_;  // our current State in the parsing FSM: an int so we can ++
   uint8_t flags_;  // the flags byte of the header ("FLG" in the RFC)
   uint16_t extra_length_;  // how much of the "extra field" we have yet to read

net/filter/gzip_source_stream.cc

@@ -116,18 +116,18 @@ base::expected<size_t, Error> GzipSourceStream::FilterData(
         DCHECK_NE(TYPE_DEFLATE, type());
 
         const size_t kGzipFooterBytes = 8;
-        const char* end = nullptr;
-        GZipHeader::Status status =
-            gzip_header_.ReadMore(input_data, input_data_size, &end);
+        size_t header_end = 0u;
+        GZipHeader::Status status = gzip_header_.ReadMore(
+            base::as_bytes(base::span(input_data, input_data_size)),
+            header_end);
         if (status == GZipHeader::INCOMPLETE_HEADER) {
           input_data += input_data_size;
           input_data_size = 0;
         } else if (status == GZipHeader::COMPLETE_HEADER) {
           // If there is a valid header, there should also be a valid footer.
           gzip_footer_bytes_left_ = kGzipFooterBytes;
-          size_t bytes_consumed = static_cast<size_t>(end - input_data);
-          input_data += bytes_consumed;
-          input_data_size -= bytes_consumed;
+          input_data += header_end;
+          input_data_size -= header_end;
           input_state_ = STATE_COMPRESSED_BODY;
         } else if (status == GZipHeader::INVALID_HEADER) {
           return base::unexpected(ERR_CONTENT_DECODING_FAILED);

net/filter/gzip_source_stream_unittest.cc

@@ -7,6 +7,8 @@
 #pragma allow_unsafe_buffers
 #endif
 
+#include "net/filter/gzip_source_stream.h"
+
 #include <string>
 #include <utility>
 
@@ -16,7 +18,7 @@
 #include "net/base/io_buffer.h"
 #include "net/base/test_completion_callback.h"
 #include "net/filter/filter_source_stream_test_util.h"
-#include "net/filter/gzip_source_stream.h"
+#include "net/filter/gzip_header.h"
 #include "net/filter/mock_source_stream.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/zlib/zlib.h"
@@ -57,6 +59,8 @@ struct GzipTestParam {
 
 }  // namespace
 
+// Note that these tests cover GZipHeader::HasGZipHeader(), to avoid duplicating
+// data passed to the method.
 class GzipSourceStreamTest : public ::testing::TestWithParam<GzipTestParam> {
  protected:
   GzipSourceStreamTest() : output_buffer_size_(GetParam().buffer_size) {}
@@ -106,6 +110,9 @@ class GzipSourceStreamTest : public ::testing::TestWithParam<GzipTestParam> {
 
   char* encoded_data() { return encoded_data_; }
   size_t encoded_data_len() { return encoded_data_len_; }
+  base::span<const uint8_t> encoded_span() {
+    return base::as_byte_span(encoded_data_);
+  }
 
   IOBuffer* output_buffer() { return output_buffer_.get(); }
   char* output_data() { return output_buffer_->data(); }
@@ -186,6 +193,7 @@ TEST_P(GzipSourceStreamTest, EmptyStream) {
   int result = ReadStream(&actual_output);
   EXPECT_EQ(OK, result);
   EXPECT_EQ("DEFLATE", stream()->Description());
+  EXPECT_FALSE(GZipHeader::HasGZipHeader(base::span<uint8_t>()));
 }
 
 TEST_P(GzipSourceStreamTest, DeflateOneBlock) {
@@ -198,6 +206,7 @@ TEST_P(GzipSourceStreamTest, DeflateOneBlock) {
   EXPECT_EQ(static_cast<int>(source_data_len()), rv);
   EXPECT_EQ(std::string(source_data(), source_data_len()), actual_output);
   EXPECT_EQ("DEFLATE", stream()->Description());
+  EXPECT_FALSE(GZipHeader::HasGZipHeader(encoded_span()));
 }
 
 TEST_P(GzipSourceStreamTest, GzipOneBloc) {
@@ -210,6 +219,7 @@ TEST_P(GzipSourceStreamTest, GzipOneBloc) {
   EXPECT_EQ(static_cast<int>(source_data_len()), rv);
   EXPECT_EQ(std::string(source_data(), source_data_len()), actual_output);
   EXPECT_EQ("GZIP", stream()->Description());
+  EXPECT_TRUE(GZipHeader::HasGZipHeader(encoded_span()));
 }
 
 TEST_P(GzipSourceStreamTest, DeflateTwoReads) {
@@ -259,6 +269,7 @@ TEST_P(GzipSourceStreamTest, MissingZlibHeader) {
   EXPECT_EQ(static_cast<int>(source_data_len()), rv);
   EXPECT_EQ(std::string(source_data(), source_data_len()), actual_output);
   EXPECT_EQ("DEFLATE", stream()->Description());
+  EXPECT_FALSE(GZipHeader::HasGZipHeader(encoded_span().first(kZlibHeaderLen)));
 }
 
 TEST_P(GzipSourceStreamTest, CorruptGzipHeader) {
@@ -273,6 +284,7 @@ TEST_P(GzipSourceStreamTest, CorruptGzipHeader) {
   int rv = ReadStream(&actual_output);
   EXPECT_EQ(ERR_CONTENT_DECODING_FAILED, rv);
   EXPECT_EQ("GZIP", stream()->Description());
+  EXPECT_FALSE(GZipHeader::HasGZipHeader(encoded_span()));
 }
 
 // This test checks that the gzip stream source works correctly on 'golden' data
@@ -295,6 +307,7 @@ TEST_P(GzipSourceStreamTest, GzipCorrectness) {
   EXPECT_EQ(static_cast<int>(strlen(kDecompressedData)), rv);
   EXPECT_EQ(kDecompressedData, actual_output);
   EXPECT_EQ("GZIP", stream()->Description());
+  EXPECT_TRUE(GZipHeader::HasGZipHeader(base::as_byte_span(kGzipData)));
 }
 
 // Same as GzipCorrectness except that last 8 bytes are removed to test that the
@@ -317,6 +330,7 @@ TEST_P(GzipSourceStreamTest, GzipCorrectnessWithoutFooter) {
   EXPECT_EQ(static_cast<int>(strlen(kDecompressedData)), rv);
   EXPECT_EQ(kDecompressedData, actual_output);
   EXPECT_EQ("GZIP", stream()->Description());
+  EXPECT_TRUE(GZipHeader::HasGZipHeader(base::as_byte_span(kGzipData)));
 }
 
 // Test with the same compressed data as the above tests, but uses deflate with
@@ -336,6 +350,7 @@ TEST_P(GzipSourceStreamTest, DeflateWithAdler32) {
   EXPECT_EQ(static_cast<int>(strlen(kDecompressedData)), rv);
   EXPECT_EQ(kDecompressedData, actual_output);
   EXPECT_EQ("DEFLATE", stream()->Description());
+  EXPECT_FALSE(GZipHeader::HasGZipHeader(base::as_byte_span(kGzipData)));
 }
 
 TEST_P(GzipSourceStreamTest, DeflateWithBadAdler32) {
@@ -349,6 +364,7 @@ TEST_P(GzipSourceStreamTest, DeflateWithBadAdler32) {
   int rv = ReadStream(&actual_output);
   EXPECT_EQ(ERR_CONTENT_DECODING_FAILED, rv);
   EXPECT_EQ("DEFLATE", stream()->Description());
+  EXPECT_FALSE(GZipHeader::HasGZipHeader(base::as_byte_span(kGzipData)));
 }
 
 TEST_P(GzipSourceStreamTest, DeflateWithoutHeaderWithAdler32) {
@@ -365,6 +381,7 @@ TEST_P(GzipSourceStreamTest, DeflateWithoutHeaderWithAdler32) {
   EXPECT_EQ(static_cast<int>(strlen(kDecompressedData)), rv);
   EXPECT_EQ(kDecompressedData, actual_output);
   EXPECT_EQ("DEFLATE", stream()->Description());
+  EXPECT_FALSE(GZipHeader::HasGZipHeader(base::as_byte_span(kGzipData)));
 }
 
 TEST_P(GzipSourceStreamTest, DeflateWithoutHeaderWithBadAdler32) {
@@ -378,6 +395,7 @@ TEST_P(GzipSourceStreamTest, DeflateWithoutHeaderWithBadAdler32) {
   int rv = ReadStream(&actual_output);
   EXPECT_EQ(ERR_CONTENT_DECODING_FAILED, rv);
   EXPECT_EQ("DEFLATE", stream()->Description());
+  EXPECT_FALSE(GZipHeader::HasGZipHeader(base::as_byte_span(kGzipData)));
 }
 
 }  // namespace net

ui/base/resource/data_pack.cc

@@ -76,14 +76,6 @@ void MaybePrintResourceId(uint16_t resource_id) {
   }
 }
 
-bool MmapHasGzipHeader(const base::MemoryMappedFile* mmap) {
-  net::GZipHeader header;
-  const char* header_end = nullptr;
-  net::GZipHeader::Status header_status = header.ReadMore(
-      reinterpret_cast<const char*>(mmap->data()), mmap->length(), &header_end);
-  return header_status == net::GZipHeader::COMPLETE_HEADER;
-}
-
 }  // namespace
 
 namespace ui {
@@ -208,7 +200,7 @@ std::unique_ptr<DataPack::DataSource> DataPack::LoadFromPathInternal(
     DLOG(ERROR) << "Failed to mmap datapack";
     return nullptr;
   }
-  if (MmapHasGzipHeader(mmap.get())) {
+  if (net::GZipHeader::HasGZipHeader(mmap->bytes())) {
     std::string_view compressed(reinterpret_cast<char*>(mmap->data()),
                                 mmap->length());
     std::string data;

ui/base/resource/resource_bundle.cc

@@ -19,6 +19,7 @@
 #include <vector>
 
 #include "base/command_line.h"
+#include "base/containers/span.h"
 #include "base/debug/alias.h"
 #include "base/files/file.h"
 #include "base/files/file_util.h"
@@ -122,15 +123,6 @@ SkBitmap CreateEmptyBitmap() {
   return bitmap;
 }
 
-// Helper function for determining whether a resource is gzipped.
-bool HasGzipHeader(std::string_view data) {
-  net::GZipHeader header;
-  const char* header_end = nullptr;
-  net::GZipHeader::Status header_status =
-      header.ReadMore(data.data(), data.length(), &header_end);
-  return header_status == net::GZipHeader::COMPLETE_HEADER;
-}
-
 // Helper function for determining whether a resource is brotli compressed.
 bool HasBrotliHeader(std::string_view data) {
   // Check that the data is brotli decoded by checking for kBrotliConst in
@@ -191,7 +183,8 @@ bool BrotliDecompress(std::string_view input, OutputBufferType output) {
 
 // Helper function for decompressing resource.
 void DecompressIfNeeded(std::string_view data, OutputBufferType output) {
-  if (!data.empty() && HasGzipHeader(data)) {
+  if (!data.empty() &&
+      net::GZipHeader::HasGZipHeader(base::as_byte_span(data))) {
     TRACE_EVENT0("ui", "DecompressIfNeeded::GzipUncompress");
     const uint32_t uncompressed_size = compression::GetUncompressedSize(data);
     bool success = compression::GzipUncompress(
@@ -694,7 +687,8 @@ base::RefCountedMemory* ResourceBundle::LoadDataResourceBytesForScale(
   if (data.empty())
     return nullptr;
 
-  if (HasGzipHeader(data) || HasBrotliHeader(data)) {
+  if (net::GZipHeader::HasGZipHeader(base::as_byte_span(data)) ||
+      HasBrotliHeader(data)) {
     base::RefCountedString* bytes_string = new base::RefCountedString();
     DecompressIfNeeded(data, &bytes_string->as_string());
     return bytes_string;
@@ -802,7 +796,7 @@ bool ResourceBundle::IsGzipped(int resource_id) const {
   if (!raw_data.data())
     return false;
 
-  return HasGzipHeader(raw_data);
+  return net::GZipHeader::HasGZipHeader(base::as_byte_span(raw_data));
 }
 
 bool ResourceBundle::IsBrotli(int resource_id) const {