Remote creates file even on 404

src/Processor/Remote.php

@@ -32,8 +32,15 @@ public function setupState(array $state): array
 
     public function copy(array $state, Result $result, int $timeLimit = PHP_INT_MAX): array
     {
+        $state['source'] = $state['source'] ?? '';
         $client = $this->getClient();
         try {
+            // Use a HEAD request to discover whether the source URL is valid.
+            // If the HTTP client throws an exception, then we can't/shouldn't
+            // allow the contents to be written to destination. See the
+            // subclass hierarchy of GuzzleException for all the cases this
+            // handles.
+            $client->head($state['source']);
             $fout = fopen($state['destination'], "w");
             $client->get($state['source'], ['sink' => $fout]);
             $result->setStatus(Result::DONE);
@@ -49,7 +56,10 @@ public function copy(array $state, Result $result, int $timeLimit = PHP_INT_MAX)
     protected function getFileSize($path): int
     {
         clearstatcache();
-        return filesize($path);
+        if ($size = @filesize($path) !== false) {
+            return $size;
+        }
+        return 0;
     }
 
     protected function getClient(): Client

test/Mock/FakeRemote.php

@@ -19,6 +19,9 @@ protected function getClient(): Client
     private function getMockHandler()
     {
         $mock = new MockHandler([
+            // Multiple copies because Remote can end up making more than one
+            // request.
+            new Response(200, ['X-Foo' => 'Bar'], 'Hello, World'),
             new Response(200, ['X-Foo' => 'Bar'], 'Hello, World'),
         ]);
         return $mock;

test/Processor/RemoteTest.php

@@ -12,17 +12,18 @@
 
 /**
  * @covers \FileFetcher\Processor\Remote
+ * @coversDefaultClass \FileFetcher\Processor\Remote
  */
 class RemoteTest extends TestCase
 {
 
     public function testCopyAFileWithRemoteProcessor()
     {
         $config = [
-            "filePath" => 'http://notreal.blah/notacsv.csv',
-            "processors" => [FakeRemote::class]
+            'filePath' => 'http://notreal.blah/notacsv.csv',
+            'processors' => [FakeRemote::class]
         ];
-        $fetcher = FileFetcher::get("1", new Memory(), $config);
+        $fetcher = FileFetcher::get('1', new Memory(), $config);
 
         $fetcher->setTimeLimit(1);
 
@@ -49,6 +50,8 @@ public function provideIsServerCompatible(): array
     /**
      * Test the \FileFetcher\Processor\Remote::isServerCompatible() method.
      *
+     * @covers ::isServerCompatible
+     *
      * @dataProvider provideIsServerCompatible
      */
     public function testIsServerCompatible($expected, $source)
@@ -57,19 +60,43 @@ public function testIsServerCompatible($expected, $source)
         $this->assertSame($expected, $processor->isServerCompatible(['source' => $source]));
     }
 
+    /**
+     * Ensure the status object contains the message from an exception.
+     */
     public function testCopyException()
     {
-        // Ensure the status object contains the message from an exception.
-        // We'll use vfsstream to mock a file system with no permissions to
-        // throw an error.
-        $root = vfsStream::setup('root', 0000);
-        $state = ['destination' => $root->url()];
+        $root = vfsStream::setup('no-write', 0000);
+
+        $remote = new Remote();
+        $result = new Result();
+
+        // State does not have a source property, so there should be a curl error.
+        $remote->copy(['destination' => $root->url() . 'file.txt'], $result);
+
+        $this->assertSame(Result::ERROR, $result->getStatus());
+        $this->assertStringContainsString('cURL error 3', $result->getError());
+    }
+
+    /**
+     * Ensure that 404 responses do not write content to disk.
+     */
+    public function test404DoesNotCreateFile()
+    {
+        $root = vfsStream::setup('test404');
+        $root_url = $root->url() . '/bad_file.csv';
+        $state = [
+            'source' => 'http://example.com/bad_file.csv',
+            'destination' => $root_url,
+        ];
 
         $remote = new Remote();
         $result = new Result();
         $remote->copy($state, $result);
 
+        // Assert that there was a 404 error.
         $this->assertSame(Result::ERROR, $result->getStatus());
-        $this->assertStringContainsString('ailed to open stream', $result->getError());
+        $this->assertStringContainsString('resulted in a `404 Not Found` response', $result->getError());
+        // Assert that the file was not created.
+        $this->assertFileDoesNotExist($root_url);
     }
 }