Bug 1197985 - Part 1 - Successfully skip ID3 tags stretching beyond the current input buffer. r=esawin, a=ritu
authorJan Henning <jh+bugzilla@buttercookie.de>
Mon, 07 Sep 2015 19:18:31 +0200
changeset 289242 9e0c1b7c00ea8310adecd54a29db9feca145579a
parent 289241 7e8f1001a66b6371c957f78d529f43a780df543f
child 289243 35db90277a7f1322a7cfb7680119afe8b6ee0150
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersesawin, ritu
bugs1197985
milestone42.0a2
Bug 1197985 - Part 1 - Successfully skip ID3 tags stretching beyond the current input buffer. r=esawin, a=ritu 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
@@ -390,20 +390,28 @@ MP3TrackDemuxer::FindNextFrame() {
 
   uint8_t buffer[BUFFER_SIZE];
   int32_t read = 0;
   const uint8_t* frameBeg = nullptr;
   const uint8_t* bufferEnd = nullptr;
 
   while (frameBeg == bufferEnd &&
          (read = Read(buffer, mOffset, BUFFER_SIZE)) > 0) {
-    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 };
   }
@@ -607,34 +615,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();
 }
@@ -779,29 +790,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;
 }
@@ -1006,30 +1018,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.