Issue #3080098 by MegaChriz, jcnventura, dharnell.a007, hugronaphor: Fixed...

Issue #3080098 by MegaChriz, jcnventura, dharnell.a007, hugronaphor: Fixed delete http fetched file in case of a 304 or if the document is empty.

src/Feeds/Fetcher/HttpFetcher.php

@@ -121,9 +121,28 @@ class HttpFetcher extends PluginBase implements ClearableInterface, FetcherInter
 
     // 304, nothing to see here.
     if ($response->getStatusCode() == Response::HTTP_NOT_MODIFIED) {
+      // Since the fetch is getting aborted, delete the downloaded file.
+      $this->fileSystem->unlink($sink);
+
       $state->setMessage($this->t('The feed has not been updated.'));
       throw new EmptyFeedException();
     }
+    else {
+      // Check if the fetched data has any content by checking the
+      // "Content-Length" header.
+      $lengths = $response->getHeader('Content-Length');
+      if (count($lengths) > 0) {
+        $length = reset($lengths);
+        if ($length < 1) {
+          // The fetched data is empty. Delete the temporary file and abort the
+          // fetch process.
+          $this->fileSystem->unlink($sink);
+
+          $state->setMessage($this->t('The feed is empty.'));
+          throw new EmptyFeedException();
+        }
+      }
+    }
 
     return new HttpFetcherResult($sink, $response->getHeaders(), $this->fileSystem);
   }

tests/modules/feeds_test_files/feeds_test_files.routing.yml

@@ -11,3 +11,10 @@ feeds_test_files.nodes_csv:
     _controller: 'Drupal\feeds_test_files\Controller\CsvController::nodes'
   requirements:
     _permission: 'access content'
+
+feeds_test_files.http304:
+  path: '/testing/feeds/304.csv'
+  defaults:
+    _controller: 'Drupal\feeds_test_files\Controller\CsvController::return304'
+  requirements:
+    _permission: 'access content'

tests/modules/feeds_test_files/src/Controller/CsvController.php

@@ -142,6 +142,7 @@ class CsvController extends ControllerBase {
 
     // Return 304 not modified if last modified.
     if ($last_modified == $if_modified_since) {
+      $response->setStatusCode(304);
       $response->headers->set('Status', '304 Not Modified');
       return $response;
     }
@@ -159,6 +160,20 @@ class CsvController extends ControllerBase {
     return $response;
   }
 
+  /**
+   * Returns a 304 response.
+   *
+   * @return \Symfony\Component\HttpFoundation\Response
+   *   A HTTP response.
+   */
+  public function return304() {
+    $response = new Response();
+    $response->headers->set('Last-Modified', gmdate(static::DATE_RFC7231, strtotime('Sun, 19 Nov 1978 05:00:00 GMT')));
+    $response->setStatusCode(304);
+    $response->headers->set('Status', '304 Not Modified');
+    return $response;
+  }
+
   /**
    * Gets the path for the specified module.
    *

tests/src/Functional/Feeds/Fetcher/HttpFetcherTest.php

@@ -3,8 +3,10 @@
 namespace Drupal\Tests\feeds\Functional\Feeds\Fetcher;
 
 use Drupal\Core\Field\FieldStorageDefinitionInterface;
+use Drupal\feeds\Exception\EmptyFeedException;
 use Drupal\feeds\FeedTypeInterface;
 use Drupal\feeds\Plugin\Type\Processor\ProcessorInterface;
+use Drupal\feeds\StateInterface;
 use Drupal\node\Entity\Node;
 use Drupal\taxonomy\Entity\Vocabulary;
 use Drupal\Tests\feeds\Functional\FeedsBrowserTestBase;
@@ -318,4 +320,60 @@ class HttpFetcherTest extends FeedsBrowserTestBase {
     $this->assertNodeCount(0);
   }
 
+  /**
+   * Tests that a downloaded file is immediately cleaned up when it is empty.
+   */
+  public function testCleanupOnEmptyFile() {
+    $feed = $this->createFeed($this->feedType->id(), [
+      'source' => $this->resourcesUrl() . '/rss/empty.rss2',
+    ]);
+
+    // Fetch only (instead of performing the whole import).
+    $e = NULL;
+    try {
+      $feed->getType()->getFetcher()->fetch($feed, $feed->getState(StateInterface::FETCH));
+    }
+    catch (EmptyFeedException $e) {
+      // There should be an exception thrown.
+    }
+
+    // Assert that temporary files got cleaned up.
+    $this->assertCountFilesInProgressDir(0, '', 'public');
+
+    // Assert that a EmptyFeedException was thrown earlier.
+    $this->assertInstanceOf(EmptyFeedException::class, $e, 'Failed asserting that a EmptyFeedException was thrown.');
+  }
+
+  /**
+   * Tests that a temporary file is immediately cleaned up on a 304.
+   */
+  public function testCleanupOn304() {
+    $feed_type = $this->createFeedTypeForCsv([
+      'guid' => 'GUID',
+      'title' => 'Title',
+    ], [
+      'fetcher' => 'http',
+      'fetcher_configuration' => [],
+    ]);
+
+    $feed = $this->createFeed($feed_type->id(), [
+      'source' => $this->getBaseUrl() . '/testing/feeds/304.csv',
+    ]);
+
+    // Fetch only (instead of performing the whole import).
+    $e = NULL;
+    try {
+      $feed->getType()->getFetcher()->fetch($feed, $feed->getState(StateInterface::FETCH));
+    }
+    catch (EmptyFeedException $e) {
+      // There should be an exception thrown.
+    }
+
+    // Assert that temporary files got cleaned up.
+    $this->assertCountFilesInProgressDir(0, '', 'public');
+
+    // Assert that a EmptyFeedException was thrown earlier.
+    $this->assertInstanceOf(EmptyFeedException::class, $e, 'Failed asserting that a EmptyFeedException was thrown.');
+  }
+
 }