Bug 1197985 - Part 1 - Successfully skip ID3 tags stretching beyond the current input buffer. r=esawin
authorJan Henning <jh+bugzilla@buttercookie.de>
Mon, 07 Sep 2015 19:18:31 +0200
changeset 261219 afc2ca41d78b41c0126efa5116fc1bce690c4365
parent 261218 b890e7641b133f42b5b8de04dee260b025557663
child 261220 772e70e430885a69dca581ac647f7c1b6d0f54cc
push id64692
push usercbook@mozilla.com
push dateTue, 08 Sep 2015 06:56:51 +0000
treeherdermozilla-inbound@ee49a5d7058c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin
bugs1197985
milestone43.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1197985 - Part 1 - Successfully skip ID3 tags stretching beyond the current input buffer. r=esawin This also slightly tightens up invalid header detection for both ID3 and MPEG frame headers.
dom/media/MP3Demuxer.cpp
dom/media/MP3Demuxer.h
--- a/dom/media/MP3Demuxer.cpp
+++ b/dom/media/MP3Demuxer.cpp
@@ -396,20 +396,28 @@ MP3TrackDemuxer::FindNextFrame() {
 
   while (frameBeg == bufferEnd) {
     if ((!mParser.FirstFrame().Length() &&
          mOffset - mParser.ID3Header().Size() > MAX_SKIPPED_BYTES) ||
         (read = Read(buffer, mOffset, BUFFER_SIZE)) == 0) {
       // This is not a valid MPEG audio stream or we've reached EOS, give up.
       break;
     }
-    MOZ_ASSERT(mOffset + read > mOffset);
+    NS_ENSURE_TRUE(mOffset + read > mOffset, MediaByteRange(0, 0));
     mOffset += read;
     bufferEnd = buffer + read;
     frameBeg = mParser.Parse(buffer, bufferEnd);
+
+    if (frameBeg > bufferEnd) {
+      // We need to skip an ID3 tag which stretches beyond the current buffer.
+      const uint32_t bytesToSkip = frameBeg - bufferEnd;
+      NS_ENSURE_TRUE(mOffset + bytesToSkip > mOffset, MediaByteRange(0, 0));
+      mOffset += bytesToSkip;
+      frameBeg = bufferEnd;
+    }
   }
 
   if (frameBeg == bufferEnd || !mParser.CurrentFrame().Length()) {
     MP3DEMUXER_LOG("FindNext() Exit frameBeg=%p bufferEnd=%p "
                 "mParser.CurrentFrame().Length()=%d ",
                 frameBeg, bufferEnd, mParser.CurrentFrame().Length());
     return { 0, 0 };
   }
@@ -613,34 +621,37 @@ FrameParser::Parse(const uint8_t* aBeg, 
 
   if (!mID3Parser.Header().Size() && !mFirstFrame.Length()) {
     // No MP3 frames have been parsed yet, look for ID3v2 headers at file begin.
     // ID3v1 tags may only be at file end.
     // TODO: should we try to read ID3 tags at end of file/mid-stream, too?
     const uint8_t* id3Beg = mID3Parser.Parse(aBeg, aEnd);
     if (id3Beg != aEnd) {
       // ID3 headers found, skip past them.
-      aBeg = id3Beg + ID3Parser::ID3Header::SIZE + mID3Parser.Header().Size();
+      aBeg = id3Beg + ID3Parser::ID3Header::SIZE + mID3Parser.Header().Size() +
+             mID3Parser.Header().FooterSize();
     }
   }
 
   while (aBeg < aEnd && !mFrame.ParseNext(*aBeg)) {
     ++aBeg;
   }
 
   if (mFrame.Length()) {
     // MP3 frame found.
     if (!mFirstFrame.Length()) {
       mFirstFrame = mFrame;
     }
     // Move to the frame header begin to allow for whole-frame parsing.
     aBeg -= FrameHeader::SIZE;
-    return aBeg;
   }
-  return aEnd;
+  // If no headers (both ID3 and MP3) have been found, this is equivalent to returning aEnd.
+  // If we have found a large ID3 tag and want to skip past it, aBeg will point past the
+  // end of the buffer, which needs to be handled by the calling function.
+  return aBeg;
 }
 
 // FrameParser::Header
 
 FrameParser::FrameHeader::FrameHeader()
 {
   Reset();
 }
@@ -785,29 +796,30 @@ FrameParser::FrameHeader::ParseNext(uint
       Reset();
     }
   }
   return IsValid();
 }
 
 bool
 FrameParser::FrameHeader::IsValid(int aPos) const {
-  if (IsValid()) {
+  if (aPos >= SIZE) {
     return true;
   }
   if (aPos == frame_header::SYNC1) {
     return Sync1() == 0xFF;
   }
   if (aPos == frame_header::SYNC2_VERSION_LAYER_PROTECTION) {
     return Sync2() == 7 &&
            RawVersion() != 1 &&
            RawLayer() != 0;
   }
   if (aPos == frame_header::BITRATE_SAMPLERATE_PADDING_PRIVATE) {
-    return RawBitrate() != 0xF;
+    return RawBitrate() != 0xF && RawBitrate() != 0 &&
+           RawSampleRate() != 3;
   }
   return true;
 }
 
 bool
 FrameParser::FrameHeader::IsValid() const {
   return mPos >= SIZE;
 }
@@ -1012,30 +1024,38 @@ ID3Parser::ID3Header::Flags() const {
   return mRaw[id3_header::FLAGS_END - id3_header::FLAGS_LEN];
 }
 
 uint32_t
 ID3Parser::ID3Header::Size() const {
   return mSize;
 }
 
+uint8_t
+ID3Parser::ID3Header::FooterSize() const {
+  if (Flags() & (1 << 4)) {
+    return SIZE;
+  }
+  return 0;
+}
+
 bool
 ID3Parser::ID3Header::ParseNext(uint8_t c) {
   if (!Update(c)) {
     Reset();
     if (!Update(c)) {
       Reset();
     }
   }
   return IsValid();
 }
 
 bool
 ID3Parser::ID3Header::IsValid(int aPos) const {
-  if (IsValid()) {
+  if (aPos >= SIZE) {
     return true;
   }
   const uint8_t c = mRaw[aPos];
   switch (aPos) {
     case 0: case 1: case 2:
       // Expecting "ID3".
       return id3_header::ID[aPos] == c;
     case 3:
--- a/dom/media/MP3Demuxer.h
+++ b/dom/media/MP3Demuxer.h
@@ -56,19 +56,22 @@ public:
 
     // The ID3 tags are versioned like this: ID3vMajorVersion.MinorVersion.
     uint8_t MajorVersion() const;
     uint8_t MinorVersion() const;
 
     // The ID3 flags field.
     uint8_t Flags() const;
 
-    // The derived size based on the provides size fields.
+    // The derived size based on the provided size fields.
     uint32_t Size() const;
 
+    // Returns the size of an ID3v2.4 footer if present and zero otherwise.
+    uint8_t FooterSize() const;
+
     // Returns whether the parsed data is a valid ID3 header up to the given
     // byte position.
     bool IsValid(int aPos) const;
 
     // Returns whether the parsed data is a complete and valid ID3 header.
     bool IsValid() const;
 
     // Parses the next provided byte.
@@ -278,17 +281,19 @@ public:
 
   // Clear the last parsed frame to allow for next frame parsing, i.e.:
   // - sets PrevFrame to CurrentFrame
   // - resets the CurrentFrame
   // - resets ID3Header if no valid header was parsed yet
   void EndFrameSession();
 
   // Parses given buffer [aBeg, aEnd) for a valid frame header.
-  // Returns begin of frame header if a frame header was found or aEnd otherwise.
+  // Returns begin of frame header if a frame header was found or a value >= aEnd otherwise.
+  // Values > aEnd indicate that additional bytes need to be skipped for jumping
+  // across an ID3 tag stretching beyond the given buffer.
   const uint8_t* Parse(const uint8_t* aBeg, const uint8_t* aEnd);
 
   // Parses given buffer [aBeg, aEnd) for a valid VBR header.
   // Returns whether a valid VBR header was found.
   bool ParseVBRHeader(const uint8_t* aBeg, const uint8_t* aEnd);
 
 private:
   // ID3 header parser.