Skip to content

Commit fa890c2

Browse files
gr8manmichalsn
andauthored
refactor(HTTP): optimize file path parsing in download() method (#10330)
* refactor(HTTP): optimize download() file path processing and add extreme filename test * Update tests/system/HTTP/ResponseTest.php Co-authored-by: Michal Sniatala <michal@sniatala.pl> * Update tests/system/HTTP/ResponseTest.php Co-authored-by: Michal Sniatala <michal@sniatala.pl> * Update tests/system/HTTP/ResponseTest.php Co-authored-by: Michal Sniatala <michal@sniatala.pl> * chore(changelog): remove download optimization entry as it is a refactoring * fix: remove duplicated temporary directory creation in test --------- Co-authored-by: Michal Sniatala <michal@sniatala.pl>
1 parent 042d365 commit fa890c2

2 files changed

Lines changed: 40 additions & 10 deletions

File tree

system/HTTP/ResponseTrait.php

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -733,20 +733,15 @@ public function download(string $filename = '', $data = '', bool $setMime = fals
733733
return null;
734734
}
735735

736-
$filepath = '';
737736
if ($data === null) {
738-
$filepath = $filename;
739-
$filename = explode('/', str_replace(DIRECTORY_SEPARATOR, '/', $filename));
740-
$filename = end($filename);
737+
$response = new DownloadResponse(basename($filename), $setMime);
738+
$response->setFilePath($filename);
739+
740+
return $response;
741741
}
742742

743743
$response = new DownloadResponse($filename, $setMime);
744-
745-
if ($filepath !== '') {
746-
$response->setFilePath($filepath);
747-
} elseif ($data !== null) {
748-
$response->setBinary($data);
749-
}
744+
$response->setBinary($data);
750745

751746
return $response;
752747
}

tests/system/HTTP/ResponseTest.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,41 @@ public function testGetDownloadResponseByFilePath(): void
494494
$this->assertSame(file_get_contents(__FILE__), $actualOutput);
495495
}
496496

497+
public function testGetDownloadResponseByExtremeFilePath(): void
498+
{
499+
$response = new Response(new App());
500+
501+
$tempDir = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'ci4_test_dir_' . bin2hex(random_bytes(8));
502+
$this->assertTrue(mkdir($tempDir));
503+
$extremeName = 'my_extreme_file_!@#$%.txt';
504+
$extremePath = $tempDir . DIRECTORY_SEPARATOR . $extremeName;
505+
506+
try {
507+
file_put_contents($extremePath, 'extreme data');
508+
509+
$actual = $response->download($extremePath, null);
510+
511+
$this->assertInstanceOf(DownloadResponse::class, $actual);
512+
$actual->buildHeaders();
513+
514+
$expectedFilename = $extremeName;
515+
$this->assertSame(
516+
'attachment; filename="' . addslashes($expectedFilename) . '"; filename*=UTF-8\'\'' . rawurlencode($expectedFilename),
517+
$actual->getHeaderLine('Content-Disposition'),
518+
);
519+
520+
ob_start();
521+
$actual->sendBody();
522+
$actualOutput = ob_get_contents();
523+
ob_end_clean();
524+
525+
$this->assertSame('extreme data', $actualOutput);
526+
} finally {
527+
@unlink($extremePath);
528+
@rmdir($tempDir);
529+
}
530+
}
531+
497532
public function testVagueDownload(): void
498533
{
499534
$response = new Response(new App());

0 commit comments

Comments
 (0)