Merge pull request #2 from instructure/2024-06-06

6.0.1

CHANGELOG

@@ -1,3 +1,8 @@
+== 6.0.1 2024-06-05
+
+Fixes:
+* Fixed crashes when using FFMPEG::IO with non-UTF-8 encoding (e.g.: when ffprobe or ffmpeg output contains non-UTF-8 byte sequences)
+
 == 6.0.0 2024-06-05
 
 Breaking Changes:

lib/ffmpeg.rb

@@ -74,7 +74,7 @@ def self.ffmpeg_binary=(bin)
   # @return [String] the path to the ffmpeg binary
   # @raise Errno::ENOENT if the ffmpeg binary cannot be found
   def self.ffmpeg_binary
-    @ffmpeg_binary || which('ffmpeg')
+    @ffmpeg_binary ||= which('ffmpeg')
   end
 
   # Safely captures the standard output and the standard error of the ffmpeg command.
@@ -83,8 +83,8 @@ def self.ffmpeg_binary
   # @raise [Errno::ENOENT] if the ffmpeg binary cannot be found
   def self.ffmpeg_capture3(*args)
     stdout, stderr, status = Open3.capture3(ffmpeg_binary, *args)
-    FFMPEG::IO.force_encoding(stdout)
-    FFMPEG::IO.force_encoding(stderr)
+    FFMPEG::IO.encode!(stdout)
+    FFMPEG::IO.encode!(stderr)
     [stdout, stderr, status]
   end
 
@@ -106,7 +106,7 @@ def self.ffmpeg_popen3(*args, &block)
   # @return [String] the path to the ffprobe binary
   # @raise Errno::ENOENT if the ffprobe binary cannot be found
   def self.ffprobe_binary
-    @ffprobe_binary || which('ffprobe')
+    @ffprobe_binary ||= which('ffprobe')
   end
 
   # Set the path of the ffprobe binary.
@@ -129,8 +129,8 @@ def self.ffprobe_binary=(bin)
   # @raise [Errno::ENOENT] if the ffprobe binary cannot be found
   def self.ffprobe_capture3(*args)
     stdout, stderr, status = Open3.capture3(ffprobe_binary, *args)
-    FFMPEG::IO.force_encoding(stdout)
-    FFMPEG::IO.force_encoding(stderr)
+    FFMPEG::IO.encode!(stdout)
+    FFMPEG::IO.encode!(stderr)
     [stdout, stderr, status]
   end
 

lib/ffmpeg/io.rb

@@ -7,18 +7,12 @@ module FFMPEG
   # The IO class is a simple wrapper around IO objects that adds a timeout
   # to all read operations and fixes encoding issues.
   class IO
-    attr_accessor :encoding, :timeout
+    attr_accessor :timeout
 
-    @encoding = 'UTF-8'
-
-    class << self
-      attr_accessor :encoding
-    end
-
-    def self.force_encoding(chunk)
+    def self.encode!(chunk)
       chunk[/test/]
     rescue ArgumentError
-      chunk.force_encoding(encoding)
+      chunk.encode!(Encoding::UTF_8, invalid: :replace, undef: :replace, replace: '?')
     end
 
     def initialize(target)
@@ -56,7 +50,7 @@ def each(&block)
     ].each do |symbol|
       define_method(symbol) do |*args|
         data = @target.send(symbol, *args)
-        self.class.force_encoding(data) unless data.nil?
+        self.class.encode!(data) unless data.nil?
         data
       end
     end
@@ -70,7 +64,7 @@ def each(&block)
         @target.send(symbol, *args) do |data|
           timer&.tick
           timer&.pause
-          block.call(self.class.force_encoding(data))
+          block.call(self.class.encode!(data))
           timer&.resume
         end
       ensure

lib/ffmpeg/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module FFMPEG
-  VERSION = '6.0.0'
+  VERSION = '6.0.1'
 end

spec/ffmpeg/media_spec.rb

@@ -99,21 +99,12 @@ module FFMPEG
           end
         end
 
-        context 'contains ISO-8859-1 characters' do
+        context 'contains ISO-8859-1 byte sequences' do
           let(:stdout_fixture_file) { 'ffprobe_iso8859.txt' }
 
           it 'should not raise an error' do
             expect { subject }.not_to raise_error
           end
-
-          context 'with IO encoding set to ISO-8859-1' do
-            before { FFMPEG::IO.encoding = 'ISO-8859-1' }
-            after { FFMPEG::IO.encoding = 'UTF-8' }
-
-            it 'should not raise an error' do
-              expect { subject }.not_to raise_error
-            end
-          end
         end
       end
     end

spec/ffmpeg_spec.rb

@@ -4,7 +4,7 @@
 
 describe FFMPEG do
   describe '.logger' do
-    after(:each) do
+    after do
       FFMPEG.logger = Logger.new(nil)
     end
 
@@ -25,13 +25,18 @@
   end
 
   describe '.ffmpeg_binary' do
-    after(:each) do
-      FFMPEG.ffmpeg_binary = nil
+    before do
+      FFMPEG.instance_variable_set(:@ffmpeg_binary, nil)
+    end
+
+    after do
+      FFMPEG.instance_variable_set(:@ffmpeg_binary, nil)
     end
 
     it 'should default to finding from path' do
       allow(FFMPEG).to receive(:which) { '/usr/local/bin/ffmpeg' }
-      expect(FFMPEG.ffmpeg_binary).to eq FFMPEG.which('ffprobe')
+      allow(File).to receive(:executable?) { true }
+      expect(FFMPEG.ffmpeg_binary).to eq FFMPEG.which('ffmpeg')
     end
 
     it 'should be assignable' do
@@ -51,12 +56,17 @@
   end
 
   describe '.ffprobe_binary' do
-    after(:each) do
-      FFMPEG.ffprobe_binary = nil
+    before do
+      FFMPEG.instance_variable_set(:@ffprobe_binary, nil)
+    end
+
+    after do
+      FFMPEG.instance_variable_set(:@ffprobe_binary, nil)
     end
 
     it 'should default to finding from path' do
       allow(FFMPEG).to receive(:which) { '/usr/local/bin/ffprobe' }
+      allow(File).to receive(:executable?) { true }
       expect(FFMPEG.ffprobe_binary).to eq FFMPEG.which('ffprobe')
     end
 
@@ -77,7 +87,7 @@
   end
 
   describe '.max_http_redirect_attempts' do
-    after(:each) do
+    after do
       FFMPEG.max_http_redirect_attempts = nil
     end