Merge pull request #30493 from jimporter/music-invalid-files-2.2

[Bug 1171202] Music app's scanning process can stall from invalid files

apps/music/js/metadata/flac.js

@@ -58,7 +58,6 @@ var FLACMetadata = (function() {
             metadata.picture.end += block.view.viewOffset;
           });
           has_picture = true;
-
         }
 
         return (!has_vorbis_comment || !has_picture);

apps/music/js/metadata/id3v2.js

@@ -44,8 +44,12 @@ var ID3v2Metadata = (function() {
           reject(err);
           return;
         }
-        parseFrames(header, moreview, metadata);
-        resolve(metadata);
+        try {
+          parseFrames(header, moreview, metadata);
+          resolve(metadata);
+        } catch(e) {
+          reject(e);
+        }
       });
     });
   }
@@ -89,7 +93,7 @@ var ID3v2Metadata = (function() {
    * @param {Object} header The header object for this ID3 tag.
    * @param {BlobView} blobview The audio file to parse.
    * @param {Metadata} metadata The (partially filled-in) metadata object.
-   * @return {Promise} A Promise returning the parsed metadata object.
+   * @return {Object} The metadata extracted from the ID3 tag.
    */
   function parseFrames(header, blobview, metadata) {
     // In id3v2.3, the "unsynchronized" flag in the tag header applies to
@@ -107,8 +111,7 @@ var ID3v2Metadata = (function() {
         // In id3v2.4, the size includes itself, i.e. the rest of the header
         // is |extended_header_size - 4|.
         extended_header_size = blobview.readID3Uint28BE() - 4;
-      }
-      else { // id3version === 3
+      } else { // id3version === 3
         // In id3v2.3, the size *excludes* itself, i.e. the rest of the header
         // is |extended_header_size|.
         extended_header_size = blobview.readUnsignedInt();
@@ -131,7 +134,6 @@ var ID3v2Metadata = (function() {
    * @param {Object} header The header object for this ID3 tag.
    * @param {BlobView} blobview The audio file being parsed.
    * @param {Metadata} metadata The (partially filled-in) metadata object.
-   * @return {Promise} A Promise returning the parsed metadata object.
    */
   function parseFrame(header, blobview, metadata) {
     var frameid, framesize, frameflags, frame_unsynchronized = false;

apps/music/js/metadata/ogg.js

@@ -80,32 +80,33 @@ var OggMetadata = (function() {
           return;
         }
 
-        // Look for a comment header from a supported codec
-        var first_byte = fullpage.readByte();
-        var valid = false;
-        switch (first_byte) {
-        case 3:
-          valid = fullpage.readASCIIText(6) === 'vorbis';
-          metadata.tag_format = 'vorbis';
-          break;
-        case 79:
-          valid = fullpage.readASCIIText(7) === 'pusTags';
-          metadata.tag_format = 'opus';
-          break;
-        }
-        if (!valid) {
-          reject('malformed ogg comment packet');
-          return;
-        }
+        try {
+          // Look for a comment header from a supported codec
+          var first_byte = fullpage.readByte();
+          var valid = false;
+          switch (first_byte) {
+          case 3:
+            valid = fullpage.readASCIIText(6) === 'vorbis';
+            metadata.tag_format = 'vorbis';
+            break;
+          case 79:
+            valid = fullpage.readASCIIText(7) === 'pusTags';
+            metadata.tag_format = 'opus';
+            break;
+          }
+          if (!valid) {
+            reject('malformed ogg comment packet');
+            return;
+          }
 
-        readAllComments(fullpage, metadata);
+          readAllComments(fullpage, metadata);
 
-        LazyLoader.load('js/metadata/vorbis_picture.js', function() {
-          VorbisPictureComment.parsePictureComment(metadata).
-            then(function(metadata) {
-              resolve(metadata);
-            });
-        });
+          LazyLoader.load('js/metadata/vorbis_picture.js', function() {
+            VorbisPictureComment.parsePictureComment(metadata).then(resolve);
+          });
+        } catch(e) {
+          reject(e);
+        }
       });
     });
   }

apps/music/test/unit/metadata/id3v2_test.js

@@ -2,6 +2,7 @@
 'use strict';
 
 require('/test/unit/metadata/utils.js');
+require('/js/metadata/id3v1.js');
 require('/js/metadata/id3v2.js');
 
 suite('id3v2 tags', function() {
@@ -236,4 +237,29 @@ suite('id3v2 tags', function() {
     });
 
   });
+
+  suite('invalid files', function() {
+
+    [2, 3, 4].forEach(function(version) {
+      suite('invalid id3v2.' + version, function() {
+
+        test('invalid frame size', function(done) {
+          var filename = '/test-data/id3v2.' + version +
+                         '-invalid-frame-size.mp3';
+          parseMetadata(filename).catch(function(e) {
+            done();
+          });
+        });
+
+        test('invalid skipped frame', function(done) {
+          var filename = '/test-data/id3v2.' + version +
+                         '-invalid-skipped-frame-size.mp3';
+          parseMetadata(filename).catch(function() {
+            done();
+          });
+        });
+      });
+    });
+
+  });
 });