Use Marcel for mimetype validation in ffprobe

app/models/master_file.rb

@@ -727,7 +727,7 @@ def saveDerivativesHash(derivative_hash)
 
   def reloadTechnicalMetadata!
     # Reset ffprobe
-    @ffprobe = Avalon::FFprobe.new(FileLocator.new(file_location).location)
+    @ffprobe = Avalon::FFprobe.new(FileLocator.new(file_location))
 
     # Formats like MP4 can be caught as both audio and video
     # so the case statement flows in the preferred order

lib/avalon/ffprobe.rb

@@ -15,10 +15,11 @@
 module Avalon
   class FFprobe
     attr_reader :json_output, :video_stream, :audio_stream
-    # @param [String] media_path the full path to the media file
-    def initialize(media_path)
+    # @param [FileLocator] media_file a file locator instance for the media file
+    def initialize(media_file)
+      return @json_output = {} unless valid_content_type?(media_file)
       ffprobe = Settings&.ffprobe&.path || 'ffprobe'
-      raw_output = `#{ffprobe} -i "#{media_path}" -v quiet -show_format -show_streams -count_frames -of json`
+      raw_output = `#{ffprobe} -i "#{media_file.location}" -v quiet -show_format -show_streams -of json`
       # $? is a variable for the exit status of the last executed process. 
       # Success == 0, any other value means the command failed in some way.
       unless $?.exitstatus == 0
@@ -28,19 +29,12 @@ def initialize(media_path)
       end
       @json_output = JSON.parse(raw_output).deep_symbolize_keys
 
-      # Plain text files should return from ffprobe as an empty hash.
-      # When the hash is not empty, the format name is 'tty' so we provide
-      # a catch here just in case.
-      return @json_output = {} if json_output[:format][:format_name] == 'tty'
-
       @video_stream = json_output[:streams].select { |stream| stream[:codec_type] == 'video' }.first
       @audio_stream = json_output[:streams].select { |stream| stream[:codec_type] == 'audio' }.first
     end
 
     def video?
-      # ffprobe treats image files as a single frame video. This sets the codec type to video,
-      # but leaves the frame count at 1. If frame count is 1, return false.
-      return true if video_stream && video_stream[:nb_read_frames].to_i > 1
+      return true if video_stream
 
       false
     end
@@ -68,5 +62,17 @@ def display_aspect_ratio
     def original_frame_size
       "#{video_stream[:width]}x#{video_stream[:height]}" if video?
     end
+
+    private
+
+    def valid_content_type? media_file
+      # Remove S3 credentials or other params from extension output
+      extension = File.extname(media_file.location)&.gsub(/[\?#].*/, '')
+      # Fall back on file extension if magic bytes fail to identify file
+      content_type = Marcel::MimeType.for media_file.reader, extension: extension
+      return true if ['audio', 'video'].any? { |type| content_type.include?(type) }
+
+      false
+    end
   end
 end

spec/lib/avalon/ffprobe_spec.rb

@@ -18,9 +18,11 @@
 describe Avalon::FFprobe do
   subject { described_class.new(test_file) }
 
-  let(:video_file) { Rails.root.join('spec', 'fixtures', 'videoshort.mp4').to_s }
-  let(:audioless_video_file) { Rails.root.join('spec', 'fixtures', 'videoshort_no_audio.mp4').to_s }
-  let(:audio_file) { Rails.root.join('spec', 'fixtures', 'jazz-performance.mp4').to_s }
+  let(:video_file) { FileLocator.new(Rails.root.join('spec', 'fixtures', 'videoshort.mp4').to_s) }
+  let(:audioless_video_file) { FileLocator.new(Rails.root.join('spec', 'fixtures', 'videoshort_no_audio.mp4').to_s) }
+  let(:audio_file) { FileLocator.new(Rails.root.join('spec', 'fixtures', 'jazz-performance.mp4').to_s) }
+  let(:image_file) { FileLocator.new(Rails.root.join('spec', 'fixtures', 'collection_poster.png').to_s) }
+  let(:text_file) { FileLocator.new(Rails.root.join('spec', 'fixtures', 'chunk_test.txt').to_s) }
 
   describe 'error handling' do
     let(:test_file) { video_file }
@@ -50,12 +52,12 @@
     end
 
     context 'with non-media files' do
-      let(:text_file) { described_class.new(Rails.root.join('spec', 'fixtures', 'chunk_test.txt')) }
-      let(:image_file) { described_class.new(Rails.root.join('spec', 'fixtures', 'collection_poster.png')) }
+      let(:text_test) { described_class.new(text_file) }
+      let(:image_test) { described_class.new(image_file) }
 
       it 'returns false' do
-        expect(text_file.video?).to be false
-        expect(image_file.video?).to be false
+        expect(text_test.video?).to be false
+        expect(image_test.video?).to be false
       end
     end
   end
@@ -86,12 +88,12 @@
     end
 
     context 'with non-media files' do
-      let(:text_file) { described_class.new(Rails.root.join('spec', 'fixtures', 'chunk_test.txt')) }
-      let(:image_file) { described_class.new(Rails.root.join('spec', 'fixtures', 'collection_poster.png')) }
+      let(:text_test) { described_class.new(text_file) }
+      let(:image_test) { described_class.new(image_file) }
 
       it 'returns false' do
-        expect(text_file.audio?).to be false
-        expect(image_file.audio?).to be false
+        expect(text_test.audio?).to be false
+        expect(image_test.audio?).to be false
       end
     end
   end
@@ -104,12 +106,12 @@
     end
 
     context 'with non-media files' do
-      let(:text_file) { described_class.new(Rails.root.join('spec', 'fixtures', 'chunk_test.txt')) }
-      let(:image_file) { described_class.new(Rails.root.join('spec', 'fixtures', 'collection_poster.png')) }
+      let(:text_test) { described_class.new(text_file) }
+      let(:image_test) { described_class.new(image_file) }
 
       it 'returns nil' do
-        expect(text_file.duration).to be nil
-        expect(image_file.duration).to be nil
+        expect(text_test.duration).to be nil
+        expect(image_test.duration).to be nil
       end
     end
   end
@@ -138,12 +140,12 @@
     end
 
     context 'with non-media files' do
-      let(:text_file) { described_class.new(Rails.root.join('spec', 'fixtures', 'chunk_test.txt')) }
-      let(:image_file) { described_class.new(Rails.root.join('spec', 'fixtures', 'collection_poster.png')) }
+      let(:text_test) { described_class.new(text_file) }
+      let(:image_test) { described_class.new(image_file) }
 
       it 'returns nil' do
-        expect(text_file.display_aspect_ratio).to be nil
-        expect(image_file.display_aspect_ratio).to be nil
+        expect(text_test.display_aspect_ratio).to be nil
+        expect(image_test.display_aspect_ratio).to be nil
       end
     end
   end
@@ -166,12 +168,12 @@
     end
 
     context 'with non-media files' do
-      let(:text_file) { described_class.new(Rails.root.join('spec', 'fixtures', 'chunk_test.txt')) }
-      let(:image_file) { described_class.new(Rails.root.join('spec', 'fixtures', 'collection_poster.png')) }
+      let(:text_test) { described_class.new(text_file) }
+      let(:image_test) { described_class.new(image_file) }
 
       it 'returns nil' do
-        expect(text_file.original_frame_size).to be nil
-        expect(image_file.original_frame_size).to be nil
+        expect(text_test.original_frame_size).to be nil
+        expect(image_test.original_frame_size).to be nil
       end
     end
   end