Merge pull request #301 from open-sausages/pulls/1.4/update-strip-var…

…iant-to-account-for-hash BUG Update FileIDHelperResolutionStrategy::stripVariant to work with hexadecimal folders
silverstripe · Jul 5, 2019 · fa61613 · fa61613
2 parents 0f50a3e + 3093ed3
commit fa61613
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 0 deletions.
diff --git a/src/FilenameParsing/FileIDHelperResolutionStrategy.php b/src/FilenameParsing/FileIDHelperResolutionStrategy.php
@@ -502,6 +502,12 @@ public function stripVariant($fileID)
         foreach ($this->resolutionFileIDHelpers as $fileIDHelper) {
             $parsedFileID = $fileIDHelper->parseFileID($fileID);
             if ($parsedFileID) {
+                if ($hash && $parsedFileID->getHash() && !$this->hasher->compare($parsedFileID->getHash(), $hash)) {
+                    // Our file ID came bundled with an Hash, we got an hash from our helper, but that hash didn't
+                    // match what we were expecting.
+                    continue;
+                }
+
                 if ($hash) {
                     $parsedFileID = $parsedFileID->setHash($hash);
                 }

diff --git a/tests/php/FilenameParsing/FileIDHelperResolutionStrategyTest.php b/tests/php/FilenameParsing/FileIDHelperResolutionStrategyTest.php
@@ -646,6 +646,7 @@ public function testGenerateVariantFileID($mainFilePath, $content, ParsedFileID
     public function listVariantParsedFiledID()
     {
         $pfid = new ParsedFileID('folder/file.txt', 'abcdef7890');
+        $ambigious = new ParsedFileID('decade1980/file.txt', 'abcdef7890');
         return [
             'Variantless Natural path' =>
                 [$pfid->setFileID('folder/file.txt')->setHash(''), 'folder/file.txt'],
@@ -669,6 +670,14 @@ public function listVariantParsedFiledID()
                     $pfid->setFileID('folder/abcdef7890/file.txt'),
                     $pfid->setFileID('folder/abcdef7890/file__variant.txt')
                 ],
+            'File in folder that could be an hash' => [
+                $ambigious->setFileID('decade1980/file.txt'),
+                $ambigious->setFileID('decade1980/file.txt'),
+            ],
+            'File variant in folder that could be an hash' => [
+                $ambigious->setFileID('decade1980/file.txt'),
+                $ambigious->setFileID('decade1980/file__variant.txt')->setVariant('variant'),
+            ]
         ];
     }
 

diff --git a/tests/php/Storage/AssetStoreTest.php b/tests/php/Storage/AssetStoreTest.php
@@ -1163,4 +1163,36 @@ private function writeDummyFiles($fsName, array $contents)
             $fs->write($path, $content);
         }
     }
+
+    public function testExist()
+    {
+        // "decade1980" could be confused for an hash because it's 10 hexa-decimal characters
+        $filename = 'decade1980/pangram.txt';
+        $content = 'The quick brown fox jumps over a lazy dog';
+        $hash = sha1($content);
+        $store = $this->getBackend();
+
+        // File haven't been created yet
+        $this->assertFalse($store->exists($filename, $hash));
+        $this->assertFalse($store->exists($filename, $hash, 'variant'));
+
+        // Create main file on protected store
+        $store->setFromString($content, $filename);
+        $this->assertTrue($store->exists($filename, $hash));
+        $this->assertFalse($store->exists($filename, $hash, 'variant'));
+
+        // Create variant on protected store
+        $store->setFromString(strtoupper($content), $filename, $hash, 'variant');
+        $this->assertTrue($store->exists($filename, $hash, 'variant'));
+
+        // Publish file to public store
+        $store->publish($filename, $hash);
+        $this->assertTrue($store->exists($filename, $hash));
+        $this->assertTrue($store->exists($filename, $hash, 'variant'));
+
+        // Files should be gone
+        $store->delete($filename, $hash);
+        $this->assertFalse($store->exists($filename, $hash));
+        $this->assertFalse($store->exists($filename, $hash, 'variant'));
+    }
 }