Fix issue with properly resolving url in http-based repository (#351)

Fix issue with properly resolving url in http-based repository

Author: benjpaddock

.codacy.yml

@@ -1,3 +1,4 @@
+---
 exclude_paths:
   - '*.md'
   - '**/tests/**'

composer.json

@@ -30,6 +30,7 @@
         "ext-zip": "*",
         "guzzlehttp/guzzle": "^7.4",
         "laravel/framework": "^8.83.10",
+        "league/uri": "^6.5",
         "phpstan/extension-installer": "^1.1",
         "phpstan/phpstan-phpunit": "^1.1"
     },

src/Exceptions/ReleaseException.php

@@ -12,4 +12,24 @@ public static function noReleaseFound(string $version): self
 
         return new self(sprintf('No release found for version "%s". Please check the repository you\'re pulling from', $version));
     }
+
+    public static function cannotExtractDownloadLink(string $pattern): self
+    {
+        return new self(sprintf('Cannot extract download/release link from source. Pattern "%s" not found.', $pattern));
+    }
+
+    public static function archiveFileNotFound(string $path): self
+    {
+        return new self(sprintf('Archive file "%s" not found.', $path));
+    }
+
+    public static function archiveNotAZipFile(string $mimeType): self
+    {
+        return new self(sprintf('File is not a zip archive. File is "%s"', $mimeType));
+    }
+
+    public static function cannotExtractArchiveFile(string $path): self
+    {
+        return new self(sprintf('Cannot open zip archive "%s"', $path));
+    }
 }

src/Models/Release.php

@@ -4,10 +4,11 @@
 
 namespace Codedge\Updater\Models;
 
+use Codedge\Updater\Exceptions\ReleaseException;
 use Codedge\Updater\Traits\SupportPrivateAccessToken;
 use Exception;
-use Illuminate\Filesystem\Filesystem;
 use Illuminate\Http\Client\Response;
+use Illuminate\Support\Facades\File;
 use Illuminate\Support\Facades\Http;
 use Illuminate\Support\Str;
 use Symfony\Component\Finder\Finder;
@@ -45,13 +46,6 @@ final class Release
      */
     private ?string $downloadUrl = null;
 
-    protected Filesystem $filesystem;
-
-    public function __construct(Filesystem $filesystem)
-    {
-        $this->filesystem = $filesystem;
-    }
-
     public function getRelease(): ?string
     {
         return $this->release;
@@ -73,8 +67,8 @@ public function setStoragePath(string $storagePath): self
     {
         $this->storagePath = $storagePath;
 
-        if (!$this->filesystem->exists($this->storagePath)) {
-            $this->filesystem->makeDirectory($this->storagePath, 493, true, true);
+        if (!File::exists($this->storagePath)) {
+            File::makeDirectory($this->storagePath, 493, true, true);
         }
 
         return $this;
@@ -135,6 +129,10 @@ public function setDownloadUrl(string $downloadUrl): self
 
     public function extract(bool $deleteSource = true): bool
     {
+        if (!File::exists($this->getStoragePath())) {
+            throw ReleaseException::archiveFileNotFound($this->getStoragePath());
+        }
+
         $extractTo = createFolderFromFile($this->getStoragePath());
         $extension = pathinfo($this->getStoragePath(), PATHINFO_EXTENSION);
 
@@ -143,30 +141,22 @@ public function extract(bool $deleteSource = true): bool
 
             // Create the final release directory
             if ($extracted && $this->createReleaseFolder() && $deleteSource) {
-                $this->filesystem->delete($this->storagePath);
+                File::delete($this->getStoragePath());
             }
 
             return true;
         } else {
-            throw new Exception('File is not a zip archive. File is '.$this->filesystem->mimeType($this->getStoragePath()).'.');
+            throw ReleaseException::archiveNotAZipFile(File::mimeType($this->getStoragePath()));
         }
     }
 
     protected function extractZip(string $extractTo): bool
     {
         $zip = new \ZipArchive();
-
-        /*
-         * @see https://bugs.php.net/bug.php?id=79296
-         */
-        if (filesize($this->getStoragePath()) === 0) {
-            $res = $zip->open($this->getStoragePath(), \ZipArchive::OVERWRITE);
-        } else {
-            $res = $zip->open($this->getStoragePath());
-        }
+        $res = $zip->open($this->getStoragePath());
 
         if ($res !== true) {
-            throw new Exception("Cannot open zip archive [{$this->getStoragePath()}]. Error: $res");
+            throw ReleaseException::cannotExtractArchiveFile($this->getStoragePath());
         }
 
         $extracted = $zip->extractTo($extractTo);
@@ -202,11 +192,11 @@ public function download(): Response
      */
     protected function createReleaseFolder(): bool
     {
-        $folders = $this->filesystem->directories(createFolderFromFile($this->getStoragePath()));
+        $folders = File::directories(createFolderFromFile($this->getStoragePath()));
 
         if (count($folders) === 1) {
             // Only one sub-folder inside extracted directory
-            $moved = $this->filesystem->moveDirectory(
+            $moved = File::moveDirectory(
                 $folders[0],
                 createFolderFromFile($this->getStoragePath()).now()->toDateString()
             );
@@ -215,14 +205,14 @@ protected function createReleaseFolder(): bool
                 return false;
             }
 
-            $this->filesystem->moveDirectory(
+            File::moveDirectory(
                 createFolderFromFile($this->getStoragePath()).now()->toDateString(),
                 createFolderFromFile($this->getStoragePath()),
                 true
             );
         }
 
-        $this->filesystem->delete($this->getStoragePath());
+        File::delete($this->getStoragePath());
 
         return true;
     }
@@ -235,20 +225,20 @@ public function isSourceAlreadyFetched(): bool
         $extractionDir = createFolderFromFile($this->getStoragePath());
 
         // Check if source archive is (probably) deleted but extracted folder is there.
-        if (!$this->filesystem->exists($this->getStoragePath())
-            && $this->filesystem->exists($extractionDir)) {
+        if (!File::exists($this->getStoragePath())
+            && File::exists($extractionDir)) {
             return true;
         }
 
         // Check if source archive is there but not extracted
-        if ($this->filesystem->exists($this->getStoragePath())
-            && !$this->filesystem->exists($extractionDir)) {
+        if (File::exists($this->getStoragePath())
+            && !File::exists($extractionDir)) {
             return true;
         }
 
         // Check if source archive and folder exists
-        if ($this->filesystem->exists($this->getStoragePath())
-            && $this->filesystem->exists($extractionDir)) {
+        if (File::exists($this->getStoragePath())
+            && File::exists($extractionDir)) {
             return true;
         }
 

src/SourceRepositoryTypes/HttpRepositoryType.php

@@ -20,6 +20,7 @@
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Str;
 use InvalidArgumentException;
+use League\Uri\Uri;
 
 class HttpRepositoryType implements SourceRepositoryTypeContract
 {
@@ -79,7 +80,7 @@ public function isNewVersionAvailable(string $currentVersion = ''): bool
     /**
      * Fetches the latest version. If you do not want the latest version, specify one and pass it.
      *
-     * @throws GuzzleException|ReleaseException
+     * @throws ReleaseException
      */
     public function fetch(string $version = ''): Release
     {
@@ -176,26 +177,41 @@ final public function getReleases(): Response
         return Http::withHeaders($headers)->get($repositoryUrl);
     }
 
-    private function extractFromHtml(string $content): Collection
+    /**
+     * @throws ReleaseException
+     */
+    public function extractFromHtml(string $content): Collection
     {
-        $format = str_replace(
-            '_VERSION_',
-            '(\d+\.\d+\.\d+)',
-            str_replace('.', '\.', $this->config['pkg_filename_format'])
-        ).'.zip';
-        $linkPattern = '<a.*href="(.*'.$format.')"';
+        $format = str_replace('_VERSION_', '(\d+\.\d+\.\d+)', $this->config['pkg_filename_format']).'.zip';
+        $linkPattern = 'a.*href="(.*'.$format.')"';
+
+        preg_match_all('<'.$linkPattern.'>i', $content, $files);
+        $files = array_filter($files);
+
+        if (count($files) === 0) {
+            throw ReleaseException::cannotExtractDownloadLink($format);
+        }
+
+        // Special handling when file version cannot be properly detected
+        if (!array_key_exists(2, $files)) {
+            foreach ($files[1] as $key=>$val) {
+                preg_match('/[a-zA-Z\-]([.\d]*)(?=\.\w+$)/', $val, $versions);
+                $files[][$key] = $versions[1];
+            }
+        }
+
+        $releaseVersions = array_combine($files[2], $files[1]);
 
-        preg_match_all('/'.$linkPattern.'/i', $content, $files);
-        $releaseVersions = $files[2];
+        $uri = Uri::createFromString($this->config['repository_url']);
+        $baseUrl = $uri->getScheme().'://'.$uri->getHost();
 
-        // Extract domain only
-        preg_match('/(?:\w+:)?\/\/[^\/]+([^?#]+)/', $this->config['repository_url'], $matches);
-        $baseUrl = preg_replace('#'.$matches[1].'#', '', $this->config['repository_url']);
+        $releases = collect($releaseVersions)->map(function ($item, $key) use ($baseUrl) {
+            $uri = Uri::createFromString($item);
+            $item = $uri->getHost() ? $item : $baseUrl.Str::start($item, '/');
 
-        $releases = collect($files[1])->map(function ($item, $key) use ($baseUrl, $releaseVersions) {
             return (object) [
-                'name'        => $releaseVersions[$key],
-                'zipball_url' => $baseUrl.$item,
+                'name'        => $key,
+                'zipball_url' => $item,
             ];
         });
 

tests/Data/releases-http.json tests/Data/Http/releases-http_gh.jsontests/Data/releases-http.json renamed to tests/Data/Http/releases-http_gh.json

@@ -107,12 +107,9 @@ public function it_cannot_fetch_releases_because_there_is_no_release(): void
         /** @var GitlabRepositoryType $gitlab */
         $gitlab = resolve(GitlabRepositoryType::class);
 
-        Http::fakeSequence()
-            ->pushResponse($this->getResponse200Type('gitlab'))
-            ->pushResponse($this->getResponseEmpty())
-            ->pushResponse($this->getResponseEmpty());
-
-        $this->assertInstanceOf(Release::class, $gitlab->fetch());
+        Http::fake([
+            '*' => $this->getResponseEmpty(),
+        ]);
 
         $this->expectException(ReleaseException::class);
         $this->expectExceptionMessage('No release found for version "latest version". Please check the repository you\'re pulling from');

tests/Data/Http/releases-http_local.json

@@ -4,11 +4,13 @@
 
 namespace Codedge\Updater\Tests\SourceRepositoryTypes;
 
+use Codedge\Updater\Exceptions\ReleaseException;
 use Codedge\Updater\Exceptions\VersionException;
 use Codedge\Updater\Models\Release;
 use Codedge\Updater\SourceRepositoryTypes\HttpRepositoryType;
 use Codedge\Updater\Tests\TestCase;
 use Exception;
+use Illuminate\Support\Facades\File;
 use Illuminate\Support\Facades\Http;
 
 final class HttpRepositoryTypeTest extends TestCase
@@ -68,9 +70,9 @@ public function it_cannot_fetch_releases_because_there_is_no_release(): void
             ->pushResponse($this->getResponse200HttpType())
             ->pushResponse($this->getResponse200ZipFile());
 
-        $this->assertInstanceOf(Release::class, $http->fetch());
+        //$this->expectException(ReleaseException::class);
+        //$this->expectExceptionMessage('Archive file "/tmp/self-updater/v/invoiceninja/invoiceninja/archive/v4.5.17.zip" not found.');
 
-        $this->expectException(Exception::class);
         $this->assertInstanceOf(Release::class, $http->fetch());
     }
 
@@ -86,6 +88,8 @@ public function it_can_fetch_http_releases(): void
             ->pushResponse($this->getResponse200HttpType())
             ->pushResponse($this->getResponse200HttpType());
 
+        File::shouldReceive('exists')->andReturnTrue();
+
         $release = $http->fetch();
 
         $this->assertInstanceOf(Release::class, $release);
@@ -169,4 +173,35 @@ public function it_can_get_new_version_available_without_version_file(): void
         $this->assertTrue($http->isNewVersionAvailable('4.5'));
         $this->assertFalse($http->isNewVersionAvailable('5.0'));
     }
+
+    /** @test */
+    public function it_can_build_releases_from_local_source(): void
+    {
+        config(['self-update.repository_types.http.repository_url' => 'http://update-server.localhost/']);
+        config(['self-update.repository_types.http.pkg_filename_format' => 'my-test-project-\d+\.\d+']);
+
+        /** @var HttpRepositoryType $http */
+        $http = $this->app->make(HttpRepositoryType::class);
+        $content = file_get_contents('tests/Data/Http/releases-http_local.json');
+
+        $collection = $http->extractFromHtml($content);
+
+        $this->assertSame('1.0', $collection->first()->name);
+        $this->assertSame('http://update-server.localhost/my534/my-test-project/-/archive/1.0/my-test-project-1.0.zip', $collection->first()->zipball_url);
+    }
+
+    /** @test */
+    public function it_can_build_releases_from_github_source(): void
+    {
+        config(['self-update.repository_types.http.repository_url' => 'https://github.com/']);
+
+        /** @var HttpRepositoryType $http */
+        $http = $this->app->make(HttpRepositoryType::class);
+        $content = file_get_contents('tests/Data/Http/releases-http_gh.json');
+
+        $collection = $http->extractFromHtml($content);
+
+        $this->assertSame('4.5.17', $collection->first()->name);
+        $this->assertSame('https://github.com/invoiceninja/invoiceninja/archive/v4.5.17.zip', $collection->first()->zipball_url);
+    }
 }

tests/SourceRepositoryTypes/GitlabRepositoryTypeTest.php

@@ -21,7 +21,7 @@ abstract class TestCase extends Orchestra
     protected array $mockedResponses = [
         'tag'    => 'releases-tag.json',
         'branch' => 'releases-branch.json',
-        'http'   => 'releases-http.json',
+        'http'   => 'releases-http_gh.json',
         'gitlab' => 'releases-gitlab.json',
     ];
 
@@ -58,7 +58,7 @@ protected function getEnvironmentSetUp($app): void
 
     protected function getResponse200HttpType(): PromiseInterface
     {
-        $stream = Utils::streamFor(fopen('tests/Data/'.$this->mockedResponses['http'], 'r'));
+        $stream = Utils::streamFor(fopen('tests/Data/Http/'.$this->mockedResponses['http'], 'r'));
         $response = $stream->getContents();
 
         return Http::response($response, 200, [