Skip to content
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.

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

dont use false as cache key for non utf8 path in normalizePath #22520

Merged
merged 1 commit into from
Sep 9, 2020

Conversation

icewind1991
Copy link
Member

since json_encode returns false if it's input isn't utf8, all non utf8 paths passed to normalizePath will currently return the same cached result.

Fixing this makes working with non utf8 storages a little bit more possible for apps

Signed-off-by: Robin Appelman robin@icewind.nl

since `json_encode` returns `false` if it's input isn't utf8, all non utf8 paths passed to normalizePath will currently return the same cached result.

Fixing this makes working with non utf8 storages a *little* bit more possible for apps

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Aug 31, 2020
@icewind1991 icewind1991 added this to the Nextcloud 20 milestone Aug 31, 2020
@faily-bot
Copy link

faily-bot bot commented Aug 31, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 32484: failure

sqlite

Show full log
There was 1 error:

1) OCA\Files_Sharing\Tests\SharedMountTest::testPermissionMovedGroupShare with data set #22 ('folder', 1, 15)
Error: Call to a member function getMountPoint() on null

/drone/src/lib/private/Share20/Manager.php:304
/drone/src/lib/private/Share20/Manager.php:705
/drone/src/apps/files_sharing/tests/TestCase.php:255
/drone/src/apps/files_sharing/tests/SharedMountTest.php:355

--

There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

mysql8.0-php7.2

Show full log
There were 2 warnings:

1) Test\Files\ViewTest::testRenameFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

2) Test\Files\ViewTest::testCopyFailDeleteTargetKeepSource
Trying to configure method "writeStream" which cannot be configured because it does not exist, has not been specified, is final, or is static

--

There was 1 failure:

1) Test\Share20\DefaultShareProviderTest::testGetSharedWithWithDeletedFile with data set #3 (1, true)
Failed asserting that actual size 1 matches expected size 0.

/drone/src/tests/lib/Share20/DefaultShareProviderTest.php:1271

@kesselb
Copy link
Contributor

kesselb commented Aug 31, 2020

Out of curiosity: Something like md5($path) would not work 🤔

@kesselb
Copy link
Contributor

kesselb commented Aug 31, 2020

Also using a json object as cache key seems super weird 🤣

This was referenced Sep 1, 2020
@juliushaertl
Copy link
Member

Out of curiosity: Something like md5($path) would not work thinking

No, since the resulting path stored in the cache depends on all the method parameters

@juliushaertl juliushaertl added 4. to release Ready to be released and/or waiting for tests to finish bug and removed 3. to review Waiting for reviews labels Sep 4, 2020
@rullzer rullzer merged commit 6895d97 into master Sep 9, 2020
@rullzer rullzer deleted the normalize-path-invalid-utf8 branch September 9, 2020 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants