Normalize file name before existence check in scanner

[PVince81]

The scanner would not find a NFD-encoded file name in an
existing file list that is normalized.

This normalizes the file name before scanning.

Fixes issues where scanning repeatedly would make NFD files flicker in
and out of existence in the file cache both with compatibility mode on and off.

Fixes #29603

• BUG: breaks the compatibility warning because the file is already pre-normalized before scanning.

TETS: For a quick test with "Local" storage, you need to remove the "isLocal" check on this line: https://github.com/nextcloud/server/blob/master/lib/private/legacy/OC_Util.php#L244

[PVince81]

maybe we could fire another event when we find a file that doesn't match normalization, the scan command can then listen to that one ?

[icewind1991]

I guess we can move the compatibility warning to the same place (compare $file before and after)

Other than that I think this should be fine

[PVince81]

I've now moved the file name encoding check to the scanner and am firing an event to trigger the warning, but only doing so if the encoding wrapper is not applied.

Fixes #29438 as well

[icewind1991]

Thinking about this again, it probably makes more sense to add normalization to the output of Encoding::opendir. That way the scanner can have it's NFD check without having to special case the compatibility wrapper.

[PVince81]

Thinking about this again, it probably makes more sense to add normalization to the output of Encoding::opendir. That way the scanner can have it's NFD check without having to special case the compatibility wrapper.

this would only work if the wrapper is applied
it would bring back the bug that rescanning twice will make entries appear and disappear again, see #29603

[icewind1991]

I mean also doing the normalization in the scanner and show the warning the filename isn't normalized when it reaches the scanner (and we should maybe also just ignore it after showing the warning?)
That way the scanner doesn't have to know about any compatibility layers (just that non normalized file names are bad), and enabling the compatibility wrapper will correctly silence the warning

[PVince81]

ok, so the way I understand, after your proposal it would work like this:

1. without encoding wrapper (compat mode off): the scanner sees the NFD encoded name and compares it with the normalized one, then triggers the new event that will display the warning

2. with encoding wrapper (compat mode on): Encoding::opendir() will pre-normalize all the directory entries, so that when the scanner sees a file name, it will already be normalized so no warning will be triggered

[PVince81]

@icewind1991 I'm looking at DirectoryWrapper but wrapSource() is protected.

How should I proceed to provide a normalized list from the opendir handle in Encoding::opendir() ?

The only implementation I found is DirectoryFilter but that one is for filtering, not pre-processing entries

[icewind1991]

It's probably easiest to first collect all files in an array, normalize them, and use IteratorDirectory to return it

[PVince81]

@icewind1991 I had to touch 3 code paths:

• getMetaData
• getDirectoryContents
• opendir

the scanner is using getDirectoryContents
to test the new EncodingDirectoryWrapper I temporarily copy-pasted Common::getDirectoryContents() into the wrapper to make it use opendir

[PVince81]

ah, and as per your advice the scanner is now skipping non-normalized names when the encoding wrapper is not in place
this means that if one disabled encoding and rescans, the entries will disappear, which looks like acceptable behavior for me

I just hope that there won't be side effects with other kinds of weird files (non-NFD) that get cleant by the normalization.
To be safer we could also use \OC_Util::normalizeUnicode() directly instead of normalizePath.
But the likeliness of seeing strange backslashes or dots in a directory listing is probably extremely low.

[PVince81]

• update unit tests of the wrapper

[icewind1991]

🚀

[PVince81]

/backport to stable23

[PVince81]

/backport to stable22

[PVince81]

/backport to stable21

[pzzszk]

Perfect - Fixed in 23.0.0. Thanks.