Skip to content

Allow app to register a download provider #34956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
artonge wants to merge 3 commits into from
Closed

Allow app to register a download provider #34956

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9c2ded5
Allow app to register a download provider
artonge Nov 7, 2022
0ec6534
Use implode instead of split
artonge Feb 2, 2023
587f731
Use explode('/', ...)
artonge Feb 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Use explode('/', ...) 
Signed-off-by: Louis Chemineau <[email protected]>
  • Loading branch information
@artonge
artonge committed Feb 2, 2023
commit 587f7315794549b6bb8e41f880f1a658a193db68
12 changes: 6 additions & 6 deletions apps/files/lib/Controller/DownloadController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public function index(string $files): ZipResponse {
return $response;
}

[$firstPrefix,] = \Sabre\Uri\split($files[0]);
[$firstPrefix,] = explode('/', $files[0], 2);
$commonPrefix = $firstPrefix;
foreach ($files as $filePath) {
$commonPrefix = $this->getCommonPrefix($filePath, $commonPrefix);
Expand Down Expand Up @@ -102,11 +102,11 @@ public function index(string $files): ZipResponse {
}

private function getCommonPrefix(string $str1, string $str2): string {
Copy link
Member

@nickvergessen nickvergessen Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the goal of this function? It looks very dangerous and I don't think it does what you think it does?

?files=['photos/sharedalbums/bob/img.png','photos/sharedalbums/bar/img.png'] has commonPrefix of photos/sharedalbums/b and then try to find notes like ob/img.png and ar/img.png?!

Copy link
Contributor Author

@artonge artonge Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's to prevent having a zip file with this kind of hierarchy:

files/
├─ userId/
│  ├─ folderName/
│  │  ├─ theFileIWant.txt

But only

theFileIWant.txt

Note that $commonPrefix is not used for searching, but only for building the zip.

However, your concern is still valid. The best way would be to be able to split the path and then compare the parts. But I am not sure that we have a proper way to do that in PHP. Any tips ?

Copy link
Member

@nickvergessen nickvergessen Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explode('/', $path) and then compare the segments?

$mbStr1 = mb_str_split($str1);
$mbStr2 = mb_str_split($str2);
$explodedStr1 = explode('/', $str1);
$explodedStr2 = explode('/', $str2);

for ($i = 0; $i < count($mbStr1); $i++) {
if ($mbStr1[$i] !== $mbStr2[$i]) {
for ($i = 0; $i < count($explodedStr1); $i++) {
if (!isset($explodedStr2[$i]) || $explodedStr1[$i] !== $explodedStr2[$i]) {
$i--;
break;
}
Expand All @@ -115,7 +115,7 @@ private function getCommonPrefix(string $str1, string $str2): string {
if ($i < 0) {
return '';
} else {
return implode(array_slice($mbStr1, 0, $i));
return implode(array_slice($explodedStr1, 0, $i));
}
}

Expand Down