Improve srt and docx file recognition

mantas-done · Apr 19, 2024 · dac4845 · dac4845
1 parent d4062b9
commit dac4845
Show file tree

Hide file tree

Showing 38 changed files with 86 additions and 48 deletions.
diff --git a/src/Code/Converters/AssConverter.php b/src/Code/Converters/AssConverter.php
@@ -6,7 +6,7 @@
 
 class AssConverter implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return preg_match('/\[Script Info\]\R/m', $file_content) === 1;
     }

diff --git a/src/Code/Converters/ConverterContract.php b/src/Code/Converters/ConverterContract.php
@@ -10,7 +10,7 @@ interface ConverterContract
      * @param string $file_content
      * @return bool
      */
-    public function canParseFileContent($file_content);
+    public function canParseFileContent($file_content, $original_file_content);
 
     /**
      * Converts file content (.srt, .stl... file content) to library's "internal format"

diff --git a/src/Code/Converters/CsvConverter.php b/src/Code/Converters/CsvConverter.php
@@ -13,7 +13,7 @@ private static function timeRegex()
         return rtrim(TxtConverter::$time_regexp, '/') . '|(\d+)/';
     }
 
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         $csv = self::csvToArray(trim($file_content));
 

diff --git a/src/Code/Converters/DfxpConverter.php b/src/Code/Converters/DfxpConverter.php
@@ -4,7 +4,7 @@
 
 class DfxpConverter implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return
             (preg_match('/xmlns="http:\/\/www\.w3\.org\/ns\/ttml"/m', $file_content) === 1 && preg_match('/xml:id="d1"/m', $file_content) === 1) // old netflix format;

diff --git a/src/Code/Converters/DocxReader.php b/src/Code/Converters/DocxReader.php
@@ -7,9 +7,25 @@
 
 class DocxReader implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
-        return strpos($file_content, 'PK') === 0 && strpos($file_content, '[Content_Types].xml') !== false;
+        if (strpos($original_file_content, 'PK') === 0 && strpos($original_file_content, '[Content_Types].xml') !== false) {
+            $tmp_file = tempnam(sys_get_temp_dir(), 'prefix_');
+            file_put_contents($tmp_file, $original_file_content);
+
+            $zip = new \ZipArchive();
+            $opened = $zip->open($tmp_file, \ZipArchive::RDONLY); // zip archive can only open real file
+            if ($opened === true) {
+                $zip->close();
+            }
+            unlink($tmp_file);
+
+            if ($opened === true) {
+                return true;
+            }
+        }
+
+        return false;
     }
 
     public function fileContentToInternalFormat($file_content, $original_file_content)

diff --git a/src/Code/Converters/EbuStlReader.php b/src/Code/Converters/EbuStlReader.php
@@ -6,7 +6,7 @@
 
 class EbuStlReader implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return substr($file_content, 3, 3) === 'STL' && is_numeric(substr($file_content, 6, 2));
     }

diff --git a/src/Code/Converters/LrcConverter.php b/src/Code/Converters/LrcConverter.php
@@ -9,7 +9,7 @@ class LrcConverter implements ConverterContract
     protected static $regexp = '/\[\s*(\d{2}:\d{2}(?:[:.]\d{1,3})?)\s*]/';
     protected static $time_offset_regexp = '/\[offset:\s*\+?(-?\d+)\s*]/s';
 
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         // only select when there is text after the timestamp
         // do not select files that have timestamp and text somewhere on the other line

diff --git a/src/Code/Converters/RtfReader.php b/src/Code/Converters/RtfReader.php
@@ -9,7 +9,7 @@
 
 class RtfReader implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return strpos($file_content, '{\rtf1') === 0;
     }

diff --git a/src/Code/Converters/SbvConverter.php b/src/Code/Converters/SbvConverter.php
@@ -4,7 +4,7 @@
 
 class SbvConverter implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return preg_match('/^\d{1,2}:\d{2}:\d{2}\.\d{3},\d{1,2}:\d{2}:\d{2}\.\d{3}\R(.*)/m', $file_content) === 1;
     }

diff --git a/src/Code/Converters/SccConverter.php b/src/Code/Converters/SccConverter.php
@@ -29,7 +29,7 @@ class SccConverter implements ConverterContract
         23.976,
     ];
 
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return preg_match('/Scenarist_SCC V1.0/', $file_content) === 1;
     }

diff --git a/src/Code/Converters/SmiConverter.php b/src/Code/Converters/SmiConverter.php
@@ -4,7 +4,7 @@
 
 class SmiConverter implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return preg_match('/<SAMI>/m', $file_content) === 1;
     }

diff --git a/src/Code/Converters/SrtConverter.php b/src/Code/Converters/SrtConverter.php
@@ -7,9 +7,9 @@
 
 class SrtConverter implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
-        return preg_match('/^0*\d?\R(\d{1,2}:\d{2}:\d{2},\d{1,3}\s*-->\s*\d{1,2}:\d{2}:\d{2},\d{1,3})\R(.+)$/m', $file_content) === 1;
+        return preg_match('/^0*\d?\R(\d{1,2}:\d{2}:\d{2}[,\.]\d{1,3}\s*-->\s*\d{1,2}:\d{2}:\d{2}[,\.]\d{1,3})\R(.+)$/m', $file_content) === 1;
     }
 
     /**

diff --git a/src/Code/Converters/StlConverter.php b/src/Code/Converters/StlConverter.php
@@ -4,7 +4,7 @@
 
 class StlConverter implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return preg_match('/^\d{2}:\d{2}:\d{2}:\d{2}\s,\s\d{2}:\d{2}:\d{2}:\d{2}\s,.+/m', $file_content) === 1;
     }

diff --git a/src/Code/Converters/SubMicroDvdConverter.php b/src/Code/Converters/SubMicroDvdConverter.php
@@ -10,7 +10,7 @@ class SubMicroDvdConverter implements ConverterContract
 
 
     static $pattern = '/(?:\{|\[)(?<start>\d+)(?:\}|\])(?:\{|\[)(?<end>\d+)(?:\}|\])(?<text>.+)/';
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return preg_match(self::$pattern, $file_content, $matches);
     }

diff --git a/src/Code/Converters/SubViewerConverter.php b/src/Code/Converters/SubViewerConverter.php
@@ -4,7 +4,7 @@
 
 class SubViewerConverter implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return preg_match('/^(\d{2}:\d{2}:\d{2}\.\d{2}),(\d{2}:\d{2}:\d{2}\.\d{2})\R(.*)$/m', $file_content) === 1;
     }

diff --git a/src/Code/Converters/TtmlConverter.php b/src/Code/Converters/TtmlConverter.php
@@ -7,7 +7,7 @@
 
 class TtmlConverter implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         $first_line = explode("\n", $file_content)[0];
 

diff --git a/src/Code/Converters/TxtConverter.php b/src/Code/Converters/TxtConverter.php
@@ -10,7 +10,7 @@ class TxtConverter implements ConverterContract
     public static $time_regexp = '/(?:\d{2}[:;])(?:\d{1,2}[:;])(?:\d{1,2}[:;])\d{1,3}|(?:\d{1,2}[:;])?(?:\d{1,2}[:;])\d{1,3}(?:[.,]\d+)?(?!\d)|\d{1,5}[.,]\d{1,3}/';
     private static $any_letter_regex = '/\p{L}/u';
 
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return self::hasText($file_content) && !Helpers::strContains($file_content, "\x00"); // not a binary file
     }

diff --git a/src/Code/Converters/TxtQuickTimeConverter.php b/src/Code/Converters/TxtQuickTimeConverter.php
@@ -5,7 +5,7 @@
 // qt.txt
 class TxtQuickTimeConverter implements ConverterContract
 {
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         return preg_match('/{QTtext}/m', $file_content) === 1;
     }

diff --git a/src/Code/Converters/VttConverter.php b/src/Code/Converters/VttConverter.php
@@ -9,7 +9,7 @@ class VttConverter implements ConverterContract
 {
     protected static $time_regexp = '((?:\d{2}:){1,2}\d{2}\.\d{3})\s+-->\s+((?:\d{2}:){1,2}\d{2}\.\d{3})';
 
-    public function canParseFileContent($file_content)
+    public function canParseFileContent($file_content, $original_file_content)
     {
         $lines = explode("\n", $file_content);
 

diff --git a/src/Code/Helpers.php b/src/Code/Helpers.php
@@ -54,14 +54,14 @@ public static function getConverterByFormat($format)
         throw new \Exception("Can't find suitable converter, for format: $format");
     }
 
-    public static function getConverterByFileContent($file_content)
+    public static function getConverterByFileContent($file_content, $original_file_content)
     {
         foreach (Subtitles::$formats as $row) {
             $class_name = $row['class'];
             $full_class_name = $class_name;
             /** @var ConverterContract $converter */
             $converter = new $full_class_name();
-            if ($converter->canParseFileContent($file_content)) {
+            if ($converter->canParseFileContent($file_content, $original_file_content)) {
                 return $converter;
             }
         }

diff --git a/src/Code/Other/DocxToText.php b/src/Code/Other/DocxToText.php
@@ -38,7 +38,8 @@ private function __construct($file_content)
         $zip = new \ZipArchive();
         $opened = $zip->open($tmp_file, \ZipArchive::RDONLY); // zip archive can only open real file
         if ($opened !== true) {
-            throw new \Exception();
+            unlink($tmp_file);
+            throw new \Exception("Can't open zip");
         }
         $this->zip = $zip;
         $this->tmp_path = $tmp_file;

diff --git a/src/Subtitles.php b/src/Subtitles.php
@@ -225,7 +225,7 @@ public static function loadFromString($string, $strict = true)
         $modified_string = Helpers::normalizeNewLines($modified_string);
         $converter->input = $modified_string;
 
-        $input_converter = Helpers::getConverterByFileContent($converter->input);
+        $input_converter = Helpers::getConverterByFileContent($converter->input, $string);
         $internal_format = $input_converter->fileContentToInternalFormat($converter->input, $string);
 
         // remove empty lines

diff --git a/tests/PublicInterfaceTest.php b/tests/PublicInterfaceTest.php
@@ -31,7 +31,7 @@ public function testConvertUsingThirdParameter()
         @unlink($temporary_srt_path);
 
         Subtitles::convert($srt_path, $temporary_srt_path, ['output_format' => 'vtt']);
-        $converter = Helpers::getConverterByFileContent(file_get_contents($temporary_srt_path));
+        $converter = Helpers::getConverterByFileContent(file_get_contents($temporary_srt_path), file_get_contents($temporary_srt_path));
         unlink($temporary_srt_path);
 
         $this->assertEquals(VttConverter::class, get_class($converter));

diff --git a/tests/formats/AssTest.php b/tests/formats/AssTest.php
@@ -16,14 +16,14 @@ class AssTest extends TestCase {
     public function testAss()
     {
         $content = file_get_contents('./tests/files/ass.ass');
-        $converter = Helpers::getConverterByFileContent($content);
+        $converter = Helpers::getConverterByFileContent($content, $content);
         $this->assertTrue(get_class($converter) === AssConverter::class);
     }
 
     public function testThisIsNotAssFormat()
     {
         $content = '[Script Info]';
-        $converter = Helpers::getConverterByFileContent($content);
+        $converter = Helpers::getConverterByFileContent($content, $content);
         $this->assertTrue(get_class($converter) !== AssConverter::class);
     }
 

diff --git a/tests/formats/CsvTest.php b/tests/formats/CsvTest.php
@@ -18,15 +18,15 @@ public function testRecognizesCsvFormat()
         $csv = 'Start,End,Text
 137.44,140.375,"Senator, we\'re making our final approach into Coruscant."
 3740.476,3742.501,"Very good, Lieutenant."';
-        $converter = Helpers::getConverterByFileContent($csv);
+        $converter = Helpers::getConverterByFileContent($csv, $csv);
         $this->assertTrue(get_class($converter) === CsvConverter::class, get_class($converter));
     }
 
     public function testDoesntSelectNonCsvFormat()
     {
         $csv = '0:00:15.1,0:00:17.4 Herkese merhaba.
 0:00:17.4,0:00:20.7 Bu videoda Microsoft office ürünlerinin';
-        $converter = Helpers::getConverterByFileContent($csv);
+        $converter = Helpers::getConverterByFileContent($csv, $csv);
         $this->assertTrue(get_class($converter) !== CsvConverter::class, get_class($converter));
     }
 
@@ -214,15 +214,15 @@ public function testDifferentElementCountShouldntBeInterpretedAsCsv()
         $csv = 'Start,End,Text
 hi
 3740.476,3742.501,"Very good, Lieutenant."';
-        $converter = Helpers::getConverterByFileContent($csv);
+        $converter = Helpers::getConverterByFileContent($csv, $csv);
         $this->assertTrue(get_class($converter) !== CsvConverter::class, get_class($converter));
     }
     public function testShouldntThrowException()
     {
         $csv = '1,a
 ' . ' ' . '
 2,b';
-        $converter = Helpers::getConverterByFileContent($csv);
+        $converter = Helpers::getConverterByFileContent($csv, $csv);
         $this->assertTrue(get_class($converter) !== CsvConverter::class, get_class($converter));
     }
 }
diff --git a/tests/formats/DfxpTest.php b/tests/formats/DfxpTest.php
@@ -15,7 +15,7 @@ class DfxpTest extends TestCase {
     public function testRecognizesDfxp()
     {
         $content = file_get_contents('./tests/files/dfxp.dfxp');
-        $converter = Helpers::getConverterByFileContent($content);
+        $converter = Helpers::getConverterByFileContent($content, $content);
         $this->assertTrue(get_class($converter) === DfxpConverter::class);
     }
 

diff --git a/tests/formats/DocxTest.php b/tests/formats/DocxTest.php
@@ -2,15 +2,16 @@
 
 namespace Formats;
 
+use Done\Subtitles\Code\Converters\DocxReader;
+use Done\Subtitles\Code\Helpers;
+use Done\Subtitles\Code\UserException;
 use Done\Subtitles\Subtitles;
 use PHPUnit\Framework\TestCase;
 use Helpers\AdditionalAssertionsTrait;
 
 class DocxTest extends TestCase
 {
-
     use AdditionalAssertionsTrait;
-
     public function testParsesDocxFile()
     {
         $content = file_get_contents('./tests/files/docx.docx');
@@ -21,4 +22,12 @@ public function testParsesDocxFile()
             ->getInternalFormat();
         $this->assertInternalFormatsEqual($expected, $actual);
     }
+
+    public function testCorruptedZip()
+    {
+        $this->expectExceptionMessage("Can't find suitable converter for the file");
+
+        $content = file_get_contents('./tests/files/corrupted.zip');
+        Helpers::getConverterByFileContent($content, $content);
+    }
 }
diff --git a/tests/formats/LrcTest.php b/tests/formats/LrcTest.php
@@ -16,7 +16,7 @@ class LrcTest extends TestCase
     public function testRecognizeLrc()
     {
         $content = file_get_contents('./tests/files/lrc.lrc');
-        $converter = Helpers::getConverterByFileContent($content);
+        $converter = Helpers::getConverterByFileContent($content, $content);
         $this->assertTrue(get_class($converter) === LrcConverter::class);
     }
 
@@ -33,7 +33,7 @@ public function testNotLrc() // let other converter handle invalid lrc
 Main Chaand Launga
 Solah Satrah Sitaare
 Sang Baandh Launga';
-        $converter = Helpers::getConverterByFileContent($content);
+        $converter = Helpers::getConverterByFileContent($content, $content);
         $this->assertTrue(get_class($converter) !== LrcConverter::class);
     }
 

diff --git a/tests/formats/SbvTest.php b/tests/formats/SbvTest.php
@@ -19,7 +19,7 @@ public function testRecognizesSbv()
 Don’t think that you can just ignore them
 because they’re not your children or relatives.
 TEXT;
-        $converter = Helpers::getConverterByFileContent($content);
+        $converter = Helpers::getConverterByFileContent($content, $content);
         $this->assertTrue(get_class($converter) === SbvConverter::class);
         $this->assertTrue(true);
     }

diff --git a/tests/formats/SccTest.php b/tests/formats/SccTest.php
@@ -16,7 +16,7 @@ class SccTest extends TestCase {
     public function testRecognizesScc()
     {
         $content = file_get_contents('./tests/files/scc.scc');
-        $converter = Helpers::getConverterByFileContent($content);
+        $converter = Helpers::getConverterByFileContent($content, $content);
         $this->assertTrue(get_class($converter) === SccConverter::class);
     }
 

diff --git a/tests/formats/SmiTest.php b/tests/formats/SmiTest.php
@@ -15,7 +15,7 @@ class SmiTest extends TestCase {
     public function testRecognizesSmi()
     {
         $content = file_get_contents('./tests/files/smi.smi');
-        $converter = Helpers::getConverterByFileContent($content);
+        $converter = Helpers::getConverterByFileContent($content, $content);
         $this->assertTrue(get_class($converter) === SmiConverter::class);
     }