bug fixes #250 and #251

thephpleague · Aug 30, 2017 · 0134684 · 0134684
1 parent c2ce6a3
commit 0134684
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,25 @@
 
 All Notable changes to `Csv` will be documented in this file
 
+## Next - TBD
+
+### Added
+
+- Nothing
+
+### Deprecated
+
+- Nothing
+
+### Fixed
+
+- Bug fixes headers from AbstractCsv::output according to RFC6266 [#250](https://github.com/thephpleague/csv/issues/250)
+- Make sure the internal source still exists before closing it [#251](https://github.com/thephpleague/csv/issues/251)
+
+### Removed
+
+- Nothing
+
 ## 9.0.1 - 2017-08-21
 
 ### Added

diff --git a/docs/9.0/connections/output.md b/docs/9.0/connections/output.md
@@ -75,7 +75,7 @@ die;
 
 use League\Csv\Reader;
 
-$reader = Reader::createFromPath('/path/to/my/file.csv');
+$reader = Reader::createFromPath('file.csv');
 $reader->output("name-for-your-file.csv");
 die;
 ~~~

diff --git a/docs/_data/project.yml b/docs/_data/project.yml
@@ -8,14 +8,14 @@ repository: 'csv'
 releases:
 #  next:
 #    version: '10.0'
-#    requires: 'PHP >= 5.5.0'
-#    latest: '8.2.1 - 2017-02-03'
+#    requires: 'PHP >= 7.3.0'
+#    latest: '10.0.0 - 2020-02-29'
 #    supported_until: 'TBD'
-#    documentation_link: '/8.0/'
+#    documentation_link: '/10.0/'
   current:
     version: '9.0'
     requires: 'PHP >= 7.0.10'
-    latest: '9.0.0 - 2017-08-18'
+    latest: '9.0.1 - 2017-08-21'
     supported_until: 'TBD'
     documentation_link: '/9.0/'
   previous:

diff --git a/docs/upgrading/changelog.md b/docs/upgrading/changelog.md
@@ -9,6 +9,6 @@ redirect_from: /changelog/
 All Notable changes to `Csv` will be documented in this file
 
 {% for release in site.github.releases %}
-## {{ release.name }}
+## {{ release.name }} - {{ release.published_at | date: "%Y-%m-%d" }}
 {{ release.body | replace:'```':'~~~' | markdownify }}
 {% endfor %}
diff --git a/src/AbstractCsv.php b/src/AbstractCsv.php
@@ -303,12 +303,8 @@ public function chunk(int $length): Generator
     public function output(string $filename = null): int
     {
         if (null !== $filename) {
-            header('Content-Type: text/csv');
-            header('Content-Transfer-Encoding: binary');
-            header('Content-Description: File Transfer');
-            header('Content-Disposition: attachment; filename="'.rawurlencode($filename).'"');
+            $this->sendHeaders($filename);
         }
-
         $input_bom = $this->getInputBOM();
         $this->document->rewind();
         $this->document->fseek(strlen($input_bom));
@@ -317,6 +313,43 @@ public function output(string $filename = null): int
         return strlen($this->output_bom) + $this->document->fpassthru();
     }
 
+    /**
+     * Send the CSV headers
+     *
+     * Adapted from Symfony\Component\HttpFoundation\ResponseHeaderBag::makeDisposition
+     *
+     * @param string|null $filename CSV disposition name
+     *
+     * @throws Exception if the submitted header is invalid according to RFC 6266
+     *
+     * @see https://tools.ietf.org/html/rfc6266#section-4.3
+     */
+    protected function sendHeaders(string $filename)
+    {
+        if (strlen($filename) != strcspn($filename, '\\/')) {
+            throw new Exception('The filename cannot contain the "/" and "\\" characters.');
+        }
+
+        $flag = FILTER_FLAG_STRIP_LOW;
+        if (strlen($filename) !== mb_strlen($filename)) {
+            $flag |= FILTER_FLAG_STRIP_HIGH;
+        }
+
+        $filenameFallback = filter_var($filename, FILTER_SANITIZE_STRING, $flag);
+        $filenameFallback = str_replace('%', '', $filenameFallback);
+
+        $disposition = sprintf('attachment; filename="%s"', str_replace('"', '\\"', $filenameFallback));
+        if ($filename !== $filenameFallback) {
+            $disposition .= sprintf("; filename*=utf-8''%s", rawurlencode($filename));
+        }
+        $disposition .= '; modification-date="'.date('r').'"';
+
+        header('Content-Type: text/csv');
+        header('Content-Transfer-Encoding: binary');
+        header('Content-Description: File Transfer');
+        header('Content-Disposition: '.$disposition);
+    }
+
     /**
      * Sets the field delimiter
      *

diff --git a/src/Stream.php b/src/Stream.php
@@ -127,7 +127,7 @@ public function __destruct()
 
         array_walk_recursive($this->filters, $walker);
 
-        if ($this->should_close_stream) {
+        if ($this->should_close_stream && is_resource($this->stream)) {
             fclose($this->stream);
         }
 

diff --git a/tests/CsvTest.php b/tests/CsvTest.php
@@ -120,15 +120,28 @@ public function testCloningIsForbidden()
     /**
      * @runInSeparateProcess
      * @covers ::output
+     * @covers ::sendHeaders
      */
     public function testOutputSize()
     {
-        $this->assertSame(60, $this->csv->output(__DIR__.'/data/test.csv'));
+        $this->assertSame(60, $this->csv->output('test.csv'));
     }
 
     /**
      * @runInSeparateProcess
      * @covers ::output
+     * @covers ::sendHeaders
+     */
+    public function testInvalidOutputFile()
+    {
+        $this->expectException(Exception::class);
+        $this->assertSame(60, $this->csv->output('invalid/file.csv'));
+    }
+
+    /**
+     * @runInSeparateProcess
+     * @covers ::output
+     * @covers ::sendHeaders
      * @covers ::createFromString
      * @covers League\Csv\Stream
      */
@@ -140,15 +153,15 @@ public function testOutputHeaders()
 
         $raw_csv = Reader::BOM_UTF8."john,doe,[email protected]\njane,doe,[email protected]\n";
         $csv = Reader::createFromString($raw_csv);
-        $csv->output('test.csv');
+        $csv->output('tést.csv');
         $headers = \xdebug_get_headers();
 
         // Due to the variety of ways the xdebug expresses Content-Type of text files,
         // we cannot count on complete string matching.
         $this->assertContains('content-type: text/csv', strtolower($headers[0]));
-        $this->assertSame($headers[1], 'Content-Transfer-Encoding: binary');
-        $this->assertSame($headers[2], 'Content-Description: File Transfer');
-        $this->assertSame($headers[3], 'Content-Disposition: attachment; filename="test.csv"');
+        $this->assertSame('Content-Transfer-Encoding: binary', $headers[1]);
+        $this->assertSame('Content-Description: File Transfer', $headers[2]);
+        $this->assertContains('Content-Disposition: attachment; filename="tst.csv"; filename*=utf-8\'\'t%C3%A9st.csv; modification-date="', $headers[3]);
     }
 
     /**