Tolerate optional chunks in WAV files

Wave files may contain optional chunks, such as a metadata one.
This CL makes WavReader tolerant to such chunks - it just ignores them.
For more details on the Wave format, please refer to
https://sites.google.com/site/musicgapi/technical-documents/wav-file-format.

Bug: webrtc:8762
Change-Id: Ie0e19dea75661808e7781f51faa1d0f0affeb3e1
Reviewed-on: https://webrtc-review.googlesource.com/c/40300
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25562}

5 files changed, 275 insertions, 111 deletions

diff --git a/common_audio/BUILD.gn b/common_audio/BUILD.gn
index 267037de53..911c0501e6 100644
--- a/common_audio/BUILD.gn
+++ b/common_audio/BUILD.gn
@@ -51,6 +51,7 @@ rtc_static_library("common_audio") {
     "../rtc_base:checks",
     "../rtc_base:gtest_prod",
     "../rtc_base:rtc_base_approved",
+    "../rtc_base:sanitizer",
     "../rtc_base/memory:aligned_array",
     "../rtc_base/memory:aligned_malloc",
     "../rtc_base/system:arch",
diff --git a/common_audio/wav_file.cc b/common_audio/wav_file.cc
index fe27c4131e..1b8bcbd923 100644
--- a/common_audio/wav_file.cc
+++ b/common_audio/wav_file.cc
@@ -13,6 +13,7 @@
 #include <errno.h>
 #include <algorithm>
 #include <cstdio>
+#include <type_traits>
 
 #include "common_audio/include/audio_util.h"
 #include "common_audio/wav_header.h"
@@ -21,23 +22,33 @@
 #include "rtc_base/system/arch.h"
 
 namespace webrtc {
+namespace {
 
 // We write 16-bit PCM WAV files.
-static const WavFormat kWavFormat = kWavFormatPcm;
-static const size_t kBytesPerSample = 2;
+constexpr WavFormat kWavFormat = kWavFormatPcm;
+static_assert(std::is_trivially_destructible<WavFormat>::value, "");
+constexpr size_t kBytesPerSample = 2;
 
 // Doesn't take ownership of the file handle and won't close it.
 class ReadableWavFile : public ReadableWav {
  public:
   explicit ReadableWavFile(FILE* file) : file_(file) {}
+  ReadableWavFile(const ReadableWavFile&) = delete;
+  ReadableWavFile& operator=(const ReadableWavFile&) = delete;
   size_t Read(void* buf, size_t num_bytes) override {
     return fread(buf, 1, num_bytes, file_);
   }
+  bool Eof() const override { return feof(file_) != 0; }
+  bool SeekForward(uint32_t num_bytes) override {
+    return fseek(file_, num_bytes, SEEK_CUR) == 0;
+  }
 
  private:
   FILE* file_;
 };
 
+}  // namespace
+
 WavReader::WavReader(const std::string& filename)
     : WavReader(rtc::OpenPlatformFileReadOnly(filename)) {}
 
diff --git a/common_audio/wav_header.cc b/common_audio/wav_header.cc
index 8df6d5c9e2..e49b7489b2 100644
--- a/common_audio/wav_header.cc
+++ b/common_audio/wav_header.cc
@@ -19,6 +19,8 @@
 #include <string>
 
 #include "rtc_base/checks.h"
+#include "rtc_base/logging.h"
+#include "rtc_base/sanitizer.h"
 #include "rtc_base/system/arch.h"
 
 namespace webrtc {
@@ -30,6 +32,11 @@ struct ChunkHeader {
 };
 static_assert(sizeof(ChunkHeader) == 8, "ChunkHeader size");
 
+struct RiffHeader {
+  ChunkHeader header;
+  uint32_t Format;
+};
+
 // We can't nest this definition in WavHeader, because VS2013 gives an error
 // on sizeof(WavHeader::fmt): "error C2070: 'unknown': illegal sizeof operand".
 struct FmtSubchunk {
@@ -44,11 +51,12 @@ struct FmtSubchunk {
 static_assert(sizeof(FmtSubchunk) == 24, "FmtSubchunk size");
 const uint32_t kFmtSubchunkSize = sizeof(FmtSubchunk) - sizeof(ChunkHeader);
 
+// Simple wav header. It does not include chunks that are not essential to read
+// audio samples.
 struct WavHeader {
-  struct {
-    ChunkHeader header;
-    uint32_t Format;
-  } riff;
+  WavHeader(const WavHeader&) = default;
+  WavHeader& operator=(const WavHeader&) = default;
+  RiffHeader riff;
   FmtSubchunk fmt;
   struct {
     ChunkHeader header;
@@ -56,6 +64,87 @@ struct WavHeader {
 };
 static_assert(sizeof(WavHeader) == kWavHeaderSize, "no padding in header");
 
+#ifdef WEBRTC_ARCH_LITTLE_ENDIAN
+static inline void WriteLE16(uint16_t* f, uint16_t x) {
+  *f = x;
+}
+static inline void WriteLE32(uint32_t* f, uint32_t x) {
+  *f = x;
+}
+static inline void WriteFourCC(uint32_t* f, char a, char b, char c, char d) {
+  *f = static_cast<uint32_t>(a) | static_cast<uint32_t>(b) << 8 |
+       static_cast<uint32_t>(c) << 16 | static_cast<uint32_t>(d) << 24;
+}
+
+static inline uint16_t ReadLE16(uint16_t x) {
+  return x;
+}
+static inline uint32_t ReadLE32(uint32_t x) {
+  return x;
+}
+static inline std::string ReadFourCC(uint32_t x) {
+  return std::string(reinterpret_cast<char*>(&x), 4);
+}
+#else
+#error "Write be-to-le conversion functions"
+#endif
+
+static inline uint32_t RiffChunkSize(size_t bytes_in_payload) {
+  return static_cast<uint32_t>(bytes_in_payload + kWavHeaderSize -
+                               sizeof(ChunkHeader));
+}
+
+static inline uint32_t ByteRate(size_t num_channels,
+                                int sample_rate,
+                                size_t bytes_per_sample) {
+  return static_cast<uint32_t>(num_channels * sample_rate * bytes_per_sample);
+}
+
+static inline uint16_t BlockAlign(size_t num_channels,
+                                  size_t bytes_per_sample) {
+  return static_cast<uint16_t>(num_channels * bytes_per_sample);
+}
+
+// Finds a chunk having the sought ID. If found, then |readable| points to the
+// first byte of the sought chunk data. If not found, the end of the file is
+// reached.
+void FindWaveChunk(ChunkHeader* chunk_header,
+                   ReadableWav* readable,
+                   const std::string sought_chunk_id) {
+  RTC_DCHECK_EQ(sought_chunk_id.size(), 4);
+  while (!readable->Eof()) {
+    if (readable->Read(chunk_header, sizeof(*chunk_header)) !=
+        sizeof(*chunk_header))
+      return;  // EOF.
+    if (ReadFourCC(chunk_header->ID) == sought_chunk_id)
+      return;  // Sought chunk found.
+    // Ignore current chunk by skipping its payload.
+    if (!readable->SeekForward(chunk_header->Size))
+      return;  // EOF or error.
+  }
+  return;  // EOF.
+}
+
+bool ReadFmtChunkData(FmtSubchunk* fmt_subchunk, ReadableWav* readable) {
+  // Reads "fmt " chunk payload.
+  if (readable->Read(&(fmt_subchunk->AudioFormat), kFmtSubchunkSize) !=
+      kFmtSubchunkSize)
+    return false;
+  const uint32_t fmt_size = ReadLE32(fmt_subchunk->header.Size);
+  if (fmt_size != kFmtSubchunkSize) {
+    // There is an optional two-byte extension field permitted to be present
+    // with PCM, but which must be zero.
+    int16_t ext_size;
+    if (kFmtSubchunkSize + sizeof(ext_size) != fmt_size)
+      return false;
+    if (readable->Read(&ext_size, sizeof(ext_size)) != sizeof(ext_size))
+      return false;
+    if (ext_size != 0)
+      return false;
+  }
+  return true;
+}
+
 }  // namespace
 
 bool CheckWavParameters(size_t num_channels,
@@ -110,47 +199,6 @@ bool CheckWavParameters(size_t num_channels,
   return true;
 }
 
-#ifdef WEBRTC_ARCH_LITTLE_ENDIAN
-static inline void WriteLE16(uint16_t* f, uint16_t x) {
-  *f = x;
-}
-static inline void WriteLE32(uint32_t* f, uint32_t x) {
-  *f = x;
-}
-static inline void WriteFourCC(uint32_t* f, char a, char b, char c, char d) {
-  *f = static_cast<uint32_t>(a) | static_cast<uint32_t>(b) << 8 |
-       static_cast<uint32_t>(c) << 16 | static_cast<uint32_t>(d) << 24;
-}
-
-static inline uint16_t ReadLE16(uint16_t x) {
-  return x;
-}
-static inline uint32_t ReadLE32(uint32_t x) {
-  return x;
-}
-static inline std::string ReadFourCC(uint32_t x) {
-  return std::string(reinterpret_cast<char*>(&x), 4);
-}
-#else
-#error "Write be-to-le conversion functions"
-#endif
-
-static inline uint32_t RiffChunkSize(size_t bytes_in_payload) {
-  return static_cast<uint32_t>(bytes_in_payload + kWavHeaderSize -
-                               sizeof(ChunkHeader));
-}
-
-static inline uint32_t ByteRate(size_t num_channels,
-                                int sample_rate,
-                                size_t bytes_per_sample) {
-  return static_cast<uint32_t>(num_channels * sample_rate * bytes_per_sample);
-}
-
-static inline uint16_t BlockAlign(size_t num_channels,
-                                  size_t bytes_per_sample) {
-  return static_cast<uint16_t>(num_channels * bytes_per_sample);
-}
-
 void WriteWavHeader(uint8_t* buf,
                     size_t num_channels,
                     int sample_rate,
@@ -160,7 +208,7 @@ void WriteWavHeader(uint8_t* buf,
   RTC_CHECK(CheckWavParameters(num_channels, sample_rate, format,
                                bytes_per_sample, num_samples));
 
-  WavHeader header;
+  auto header = rtc::MsanUninitialized<WavHeader>({});
   const size_t bytes_in_payload = bytes_per_sample * num_samples;
 
   WriteFourCC(&header.riff.header.ID, 'R', 'I', 'F', 'F');
@@ -192,25 +240,38 @@ bool ReadWavHeader(ReadableWav* readable,
                    WavFormat* format,
                    size_t* bytes_per_sample,
                    size_t* num_samples) {
-  WavHeader header;
-  if (readable->Read(&header, kWavHeaderSize - sizeof(header.data)) !=
-      kWavHeaderSize - sizeof(header.data))
+  auto header = rtc::MsanUninitialized<WavHeader>({});
+
+  // Read RIFF chunk.
+  if (readable->Read(&header.riff, sizeof(header.riff)) != sizeof(header.riff))
+    return false;
+  if (ReadFourCC(header.riff.header.ID) != "RIFF")
+    return false;
+  if (ReadFourCC(header.riff.Format) != "WAVE")
     return false;
 
-  const uint32_t fmt_size = ReadLE32(header.fmt.header.Size);
-  if (fmt_size != kFmtSubchunkSize) {
-    // There is an optional two-byte extension field permitted to be present
-    // with PCM, but which must be zero.
-    int16_t ext_size;
-    if (kFmtSubchunkSize + sizeof(ext_size) != fmt_size)
-      return false;
-    if (readable->Read(&ext_size, sizeof(ext_size)) != sizeof(ext_size))
-      return false;
-    if (ext_size != 0)
-      return false;
+  // Find "fmt " and "data" chunks. While the official Wave file specification
+  // does not put requirements on the chunks order, it is uncommon to find the
+  // "data" chunk before the "fmt " one. The code below fails if this is not the
+  // case.
+  FindWaveChunk(&header.fmt.header, readable, "fmt ");
+  if (ReadFourCC(header.fmt.header.ID) != "fmt ") {
+    RTC_LOG(LS_ERROR) << "Cannot find 'fmt ' chunk.";
+    return false;
+  }
+  if (!ReadFmtChunkData(&header.fmt, readable)) {
+    RTC_LOG(LS_ERROR) << "Cannot read 'fmt ' chunk.";
+    return false;
+  }
+  if (readable->Eof()) {
+    RTC_LOG(LS_ERROR) << "'fmt ' chunk placed after 'data' chunk.";
+    return false;
   }
-  if (readable->Read(&header.data, sizeof(header.data)) != sizeof(header.data))
+  FindWaveChunk(&header.data.header, readable, "data");
+  if (ReadFourCC(header.data.header.ID) != "data") {
+    RTC_LOG(LS_ERROR) << "Cannot find 'data' chunk.";
     return false;
+  }
 
   // Parse needed fields.
   *format = static_cast<WavFormat>(ReadLE16(header.fmt.AudioFormat));
@@ -222,16 +283,6 @@ bool ReadWavHeader(ReadableWav* readable,
     return false;
   *num_samples = bytes_in_payload / *bytes_per_sample;
 
-  // Sanity check remaining fields.
-  if (ReadFourCC(header.riff.header.ID) != "RIFF")
-    return false;
-  if (ReadFourCC(header.riff.Format) != "WAVE")
-    return false;
-  if (ReadFourCC(header.fmt.header.ID) != "fmt ")
-    return false;
-  if (ReadFourCC(header.data.header.ID) != "data")
-    return false;
-
   if (ReadLE32(header.riff.header.Size) < RiffChunkSize(bytes_in_payload))
     return false;
   if (ReadLE32(header.fmt.ByteRate) !=
diff --git a/common_audio/wav_header.h b/common_audio/wav_header.h
index 872d3abce5..a519ba556f 100644
--- a/common_audio/wav_header.h
+++ b/common_audio/wav_header.h
@@ -21,8 +21,11 @@ static const size_t kWavHeaderSize = 44;
 class ReadableWav {
  public:
   // Returns the number of bytes read.
-  size_t virtual Read(void* buf, size_t num_bytes) = 0;
-  virtual ~ReadableWav() {}
+  virtual size_t Read(void* buf, size_t num_bytes) = 0;
+  // Returns true if the end-of-file has been reached.
+  virtual bool Eof() const = 0;
+  virtual bool SeekForward(uint32_t num_bytes) = 0;
+  virtual ~ReadableWav() = default;
 };
 
 enum WavFormat {
diff --git a/common_audio/wav_header_unittest.cc b/common_audio/wav_header_unittest.cc
index b7169b53c8..2e2eda5a6f 100644
--- a/common_audio/wav_header_unittest.cc
+++ b/common_audio/wav_header_unittest.cc
@@ -19,12 +19,6 @@ namespace webrtc {
 // Doesn't take ownership of the buffer.
 class ReadableWavBuffer : public ReadableWav {
  public:
-  ReadableWavBuffer(const uint8_t* buf, size_t size)
-      : buf_(buf),
-        size_(size),
-        pos_(0),
-        buf_exhausted_(false),
-        check_read_size_(true) {}
   ReadableWavBuffer(const uint8_t* buf, size_t size, bool check_read_size)
       : buf_(buf),
         size_(size),
@@ -57,6 +51,27 @@ class ReadableWavBuffer : public ReadableWav {
     return num_bytes;
   }
 
+  bool Eof() const override { return pos_ == size_; }
+
+  bool SeekForward(uint32_t num_bytes) override {
+    // Verify we don't try to read outside of a properly sized header.
+    if (size_ >= kWavHeaderSize)
+      EXPECT_GE(size_, pos_ + num_bytes);
+    EXPECT_FALSE(buf_exhausted_);
+
+    const size_t bytes_remaining = size_ - pos_;
+    if (num_bytes > bytes_remaining) {
+      // Error: cannot seek beyond EOF.
+      return false;
+    }
+    if (num_bytes == bytes_remaining) {
+      // There should not be another read attempt after this point.
+      buf_exhausted_ = true;
+    }
+    pos_ += num_bytes;
+    return true;
+  }
+
  private:
   const uint8_t* buf_;
   const size_t size_;
@@ -103,7 +118,7 @@ TEST(WavHeaderTest, ReadWavHeaderWithErrors) {
   // invalid field is indicated in the array name, and in the comments with
   // *BAD*.
   {
-    static const uint8_t kBadRiffID[] = {
+    constexpr uint8_t kBadRiffID[] = {
         // clang-format off
         // clang formatting doesn't respect inline comments.
       'R', 'i', 'f', 'f',  // *BAD*
@@ -121,12 +136,13 @@ TEST(WavHeaderTest, ReadWavHeaderWithErrors) {
       0x99, 0xd0, 0x5b, 0x07,  // size of payload: 123457689
         // clang-format on
     };
-    ReadableWavBuffer r(kBadRiffID, sizeof(kBadRiffID));
+    ReadableWavBuffer r(kBadRiffID, sizeof(kBadRiffID),
+                        /*check_read_size=*/false);
     EXPECT_FALSE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
                                &bytes_per_sample, &num_samples));
   }
   {
-    static const uint8_t kBadBitsPerSample[] = {
+    constexpr uint8_t kBadBitsPerSample[] = {
         // clang-format off
         // clang formatting doesn't respect inline comments.
       'R', 'I', 'F', 'F',
@@ -144,12 +160,13 @@ TEST(WavHeaderTest, ReadWavHeaderWithErrors) {
       0x99, 0xd0, 0x5b, 0x07,  // size of payload: 123457689
         // clang-format on
     };
-    ReadableWavBuffer r(kBadBitsPerSample, sizeof(kBadBitsPerSample));
+    ReadableWavBuffer r(kBadBitsPerSample, sizeof(kBadBitsPerSample),
+                        /*check_read_size=*/true);
     EXPECT_FALSE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
                                &bytes_per_sample, &num_samples));
   }
   {
-    static const uint8_t kBadByteRate[] = {
+    constexpr uint8_t kBadByteRate[] = {
         // clang-format off
         // clang formatting doesn't respect inline comments.
       'R', 'I', 'F', 'F',
@@ -167,12 +184,13 @@ TEST(WavHeaderTest, ReadWavHeaderWithErrors) {
       0x99, 0xd0, 0x5b, 0x07,  // size of payload: 123457689
         // clang-format on
     };
-    ReadableWavBuffer r(kBadByteRate, sizeof(kBadByteRate));
+    ReadableWavBuffer r(kBadByteRate, sizeof(kBadByteRate),
+                        /*check_read_size=*/true);
     EXPECT_FALSE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
                                &bytes_per_sample, &num_samples));
   }
   {
-    static const uint8_t kBadFmtHeaderSize[] = {
+    constexpr uint8_t kBadFmtHeaderSize[] = {
         // clang-format off
         // clang formatting doesn't respect inline comments.
       'R', 'I', 'F', 'F',
@@ -191,12 +209,13 @@ TEST(WavHeaderTest, ReadWavHeaderWithErrors) {
       0x99, 0xd0, 0x5b, 0x07,  // size of payload: 123457689
         // clang-format on
     };
-    ReadableWavBuffer r(kBadFmtHeaderSize, sizeof(kBadFmtHeaderSize), false);
+    ReadableWavBuffer r(kBadFmtHeaderSize, sizeof(kBadFmtHeaderSize),
+                        /*check_read_size=*/false);
     EXPECT_FALSE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
                                &bytes_per_sample, &num_samples));
   }
   {
-    static const uint8_t kNonZeroExtensionField[] = {
+    constexpr uint8_t kNonZeroExtensionField[] = {
         // clang-format off
         // clang formatting doesn't respect inline comments.
       'R', 'I', 'F', 'F',
@@ -216,12 +235,12 @@ TEST(WavHeaderTest, ReadWavHeaderWithErrors) {
         // clang-format on
     };
     ReadableWavBuffer r(kNonZeroExtensionField, sizeof(kNonZeroExtensionField),
-                        false);
+                        /*check_read_size=*/false);
     EXPECT_FALSE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
                                &bytes_per_sample, &num_samples));
   }
   {
-    static const uint8_t kMissingDataChunk[] = {
+    constexpr uint8_t kMissingDataChunk[] = {
         // clang-format off
         // clang formatting doesn't respect inline comments.
       'R', 'I', 'F', 'F',
@@ -237,12 +256,13 @@ TEST(WavHeaderTest, ReadWavHeaderWithErrors) {
       8, 0,  // bits per sample: 1 * 8
         // clang-format on
     };
-    ReadableWavBuffer r(kMissingDataChunk, sizeof(kMissingDataChunk));
+    ReadableWavBuffer r(kMissingDataChunk, sizeof(kMissingDataChunk),
+                        /*check_read_size=*/true);
     EXPECT_FALSE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
                                &bytes_per_sample, &num_samples));
   }
   {
-    static const uint8_t kMissingFmtAndDataChunks[] = {
+    constexpr uint8_t kMissingFmtAndDataChunks[] = {
         // clang-format off
         // clang formatting doesn't respect inline comments.
       'R', 'I', 'F', 'F',
@@ -251,7 +271,8 @@ TEST(WavHeaderTest, ReadWavHeaderWithErrors) {
         // clang-format on
     };
     ReadableWavBuffer r(kMissingFmtAndDataChunks,
-                        sizeof(kMissingFmtAndDataChunks));
+                        sizeof(kMissingFmtAndDataChunks),
+                        /*check_read_size=*/true);
     EXPECT_FALSE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
                                &bytes_per_sample, &num_samples));
   }
@@ -259,11 +280,11 @@ TEST(WavHeaderTest, ReadWavHeaderWithErrors) {
 
 // Try writing and reading a valid WAV header and make sure it looks OK.
 TEST(WavHeaderTest, WriteAndReadWavHeader) {
-  static const int kSize = 4 + kWavHeaderSize + 4;
+  constexpr int kSize = 4 + kWavHeaderSize + 4;
   uint8_t buf[kSize];
   memset(buf, 0xa4, sizeof(buf));
   WriteWavHeader(buf + 4, 17, 12345, kWavFormatALaw, 1, 123457689);
-  static const uint8_t kExpectedBuf[] = {
+  constexpr uint8_t kExpectedBuf[] = {
       // clang-format off
       // clang formatting doesn't respect inline comments.
     0xa4, 0xa4, 0xa4, 0xa4,  // untouched bytes before header
@@ -291,7 +312,8 @@ TEST(WavHeaderTest, WriteAndReadWavHeader) {
   WavFormat format = kWavFormatPcm;
   size_t bytes_per_sample = 0;
   size_t num_samples = 0;
-  ReadableWavBuffer r(buf + 4, sizeof(buf) - 8);
+  ReadableWavBuffer r(buf + 4, sizeof(buf) - 8,
+                      /*check_read_size=*/true);
   EXPECT_TRUE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
                             &bytes_per_sample, &num_samples));
   EXPECT_EQ(17u, num_channels);
@@ -303,24 +325,67 @@ TEST(WavHeaderTest, WriteAndReadWavHeader) {
 
 // Try reading an atypical but valid WAV header and make sure it's parsed OK.
 TEST(WavHeaderTest, ReadAtypicalWavHeader) {
-  static const uint8_t kBuf[] = {
+  constexpr uint8_t kBuf[] = {
       // clang-format off
       // clang formatting doesn't respect inline comments.
     'R', 'I', 'F', 'F',
-    0x3d, 0xd1, 0x5b, 0x07,  // size of whole file - 8 + an extra 128 bytes of
-                             // "metadata": 123457689 + 44 - 8 + 128. (atypical)
+    0xbf, 0xd0, 0x5b, 0x07,  // Size of whole file - 8 + extra 2 bytes of zero
+                             // extension: 123457689 + 44 - 8 + 2 (atypical).
     'W', 'A', 'V', 'E',
     'f', 'm', 't', ' ',
-    18, 0, 0, 0,  // size of fmt block (with an atypical extension size field)
-    6, 0,  // format: A-law (6)
-    17, 0,  // channels: 17
-    0x39, 0x30, 0, 0,  // sample rate: 12345
-    0xc9, 0x33, 0x03, 0,  // byte rate: 1 * 17 * 12345
-    17, 0,  // block align: NumChannels * BytesPerSample
-    8, 0,  // bits per sample: 1 * 8
-    0, 0,  // zero extension size field (atypical)
+    18, 0, 0, 0,             // Size of fmt block (with an atypical extension
+                             // size field).
+    6, 0,                    // Format: A-law (6).
+    17, 0,                   // Channels: 17.
+    0x39, 0x30, 0, 0,        // Sample rate: 12345.
+    0xc9, 0x33, 0x03, 0,     // Byte rate: 1 * 17 * 12345.
+    17, 0,                   // Block align: NumChannels * BytesPerSample.
+    8, 0,                    // Bits per sample: 1 * 8.
+    0, 0,                    // Zero extension size field (atypical).
     'd', 'a', 't', 'a',
-    0x99, 0xd0, 0x5b, 0x07,  // size of payload: 123457689
+    0x99, 0xd0, 0x5b, 0x07,  // Size of payload: 123457689.
+      // clang-format on
+  };
+
+  size_t num_channels = 0;
+  int sample_rate = 0;
+  WavFormat format = kWavFormatPcm;
+  size_t bytes_per_sample = 0;
+  size_t num_samples = 0;
+  ReadableWavBuffer r(kBuf, sizeof(kBuf), /*check_read_size=*/true);
+  EXPECT_TRUE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
+                            &bytes_per_sample, &num_samples));
+  EXPECT_EQ(17u, num_channels);
+  EXPECT_EQ(12345, sample_rate);
+  EXPECT_EQ(kWavFormatALaw, format);
+  EXPECT_EQ(1u, bytes_per_sample);
+  EXPECT_EQ(123457689u, num_samples);
+}
+
+// Try reading a valid WAV header which contains an optional chunk and make sure
+// it's parsed OK.
+TEST(WavHeaderTest, ReadWavHeaderWithOptionalChunk) {
+  constexpr uint8_t kBuf[] = {
+      // clang-format off
+      // clang formatting doesn't respect inline comments.
+    'R', 'I', 'F', 'F',
+    0xcd, 0xd0, 0x5b, 0x07,  // Size of whole file - 8 + an extra 16 bytes of
+                             // "metadata" (8 bytes header, 16 bytes payload):
+                             // 123457689 + 44 - 8 + 16.
+    'W', 'A', 'V', 'E',
+    'f', 'm', 't', ' ',
+    16, 0, 0, 0,             // Size of fmt block.
+    6, 0,                    // Format: A-law (6).
+    17, 0,                   // Channels: 17.
+    0x39, 0x30, 0, 0,        // Sample rate: 12345.
+    0xc9, 0x33, 0x03, 0,     // Byte rate: 1 * 17 * 12345.
+    17, 0,                   // Block align: NumChannels * BytesPerSample.
+    8, 0,                    // Bits per sample: 1 * 8.
+    'L', 'I', 'S', 'T',      // Metadata chunk ID.
+    16, 0, 0, 0,             // Metadata chunk payload size.
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // Metadata (16 bytes).
+    'd', 'a', 't', 'a',
+    0x99, 0xd0, 0x5b, 0x07,  // Size of payload: 123457689.
       // clang-format on
   };
 
@@ -329,7 +394,7 @@ TEST(WavHeaderTest, ReadAtypicalWavHeader) {
   WavFormat format = kWavFormatPcm;
   size_t bytes_per_sample = 0;
   size_t num_samples = 0;
-  ReadableWavBuffer r(kBuf, sizeof(kBuf));
+  ReadableWavBuffer r(kBuf, sizeof(kBuf), /*check_read_size=*/true);
   EXPECT_TRUE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
                             &bytes_per_sample, &num_samples));
   EXPECT_EQ(17u, num_channels);
@@ -339,4 +404,37 @@ TEST(WavHeaderTest, ReadAtypicalWavHeader) {
   EXPECT_EQ(123457689u, num_samples);
 }
 
+// Try reading an invalid WAV header which has the the data chunk before the
+// format one and make sure it's not parsed.
+TEST(WavHeaderTest, ReadWavHeaderWithDataBeforeFormat) {
+  constexpr uint8_t kBuf[] = {
+      // clang-format off
+      // clang formatting doesn't respect inline comments.
+    'R', 'I', 'F', 'F',
+    52,  0,   0,   0,    // Size of whole file - 8: 16 + 44 - 8.
+    'W', 'A', 'V', 'E',
+    'd', 'a', 't', 'a',
+    16, 0, 0, 0,         // Data chunk payload size.
+    0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,  // Data 16 bytes.
+    'f', 'm', 't', ' ',
+    16,  0,   0,   0,    // Size of fmt block.
+    6,   0,              // Format: A-law (6).
+    1,   0,              // Channels: 1.
+    60,  0,   0,   0,    // Sample rate: 60.
+    60,  0,   0,   0,    // Byte rate: 1 * 1 * 60.
+    1,   0,              // Block align: NumChannels * BytesPerSample.
+    8,   0,              // Bits per sample: 1 * 8.
+      // clang-format on
+  };
+
+  size_t num_channels = 0;
+  int sample_rate = 0;
+  WavFormat format = kWavFormatPcm;
+  size_t bytes_per_sample = 0;
+  size_t num_samples = 0;
+  ReadableWavBuffer r(kBuf, sizeof(kBuf), /*check_read_size=*/false);
+  EXPECT_FALSE(ReadWavHeader(&r, &num_channels, &sample_rate, &format,
+                             &bytes_per_sample, &num_samples));
+}
+
 }  // namespace webrtc