Skip to content

Commit 331984a

Browse files
robertvokacrobertvokac
committed
feat(io-isolated-storage): a directory-qualified search pattern enumerates that directory (#2209)
IsolatedStorageFile::GetFileNames("sub/*") now lists sub's files. Both doors used to iterate the store root and match the WHOLE pattern against a bare filename, so it returned nothing although sub/nested.dat existed. THE TICKET'S BLOCKER IS SIMPLY GONE. It recorded that ".NET exact contract for a directory-qualified pattern cannot be established here because the /rv reference tree is absent". /rv is present, and .NET states the contract in its OWN comments, directly above each method: // foo\abc*.txt will give all abc*.txt files in foo directory // foo\data* will give all directory names in foo directory that starts with data Both delegate to Directory.EnumerateFiles(RootDirectory, searchPattern), and FileSystemEnumerableFactory.NormalizeInputs:45-56 does the splitting: at the LAST separator, joining the directory half onto the root and matching only the final segment. A trailing separator leaves an empty expression that becomes "*", which that function's own comment spells out. THE RESULT STAYS A BARE NAME, not a sub-path -- .NET maps each hit through Path.GetFileName (IsolatedStorageFile.cs:177) precisely because the store exists to hide its own root, so returning "sub/nested.dat" would leak the layout the type is there to conceal. ONE DELIBERATE NARROWING, ON A SECURITY BOUNDARY. .NET does NOT run the search pattern through its containment helper: GetFileNames and GetDirectoryNames are the only two doors on IsolatedStorageFile that bypass GetFullPath, so in .NET GetFileNames("../*") escapes the store and lists its parent. This port resolves the directory half through the same fullPath() every other door uses. Reproducing .NET here would mean opening a confinement hole to match a reference that has one. The port is more RESTRICTIVE, which is the direction SA-8 does not reach, and a test pins the asymmetry so it stays a decision rather than becoming an accident. Containment is about where a path LANDS, so "a/../a/*" is still fine. A SECOND, OLDER DIVERGENCE IS RECORDED RATHER THAN INTRODUCED. .NET rejects a rooted pattern outright (Arg_Path2IsRooted, FileSystemEnumerableFactory.cs:29-30). This port's fullPath() strips leading separators at EVERY door, so "/x" has always meant "x relative to the store"; rejecting it only in the pattern would make the type inconsistent with itself, which is worse than a divergence that is uniform. Both rows are pinned. My first cut of the test asserted .NET's behaviour here and was wrong -- the port's own convention is what it should have asserted. Two cases added (IsolatedStorage 58 -> 60). Five mutations, all caught: never split; split at the FIRST separator; return the sub-path instead of the bare name; skip the confinement check; a trailing separator no longer means "*". Downstream, measured per SA-2 condition 5: mobile-eggbert uses IsolatedStorageFile in one file and calls only GetUserStoreForApplication, never either enumeration door; cna's GetFileNames/GetDirectoryNames are its own StorageContainer's and are untouched. Zero affected sites in both. Neither was modified. Gate: 17,306 run, 17,306 passed, 0 failed, 0 skipped across 38 executables, GREEN. docs/Migration-IsolatedStorageDirectoryQualifiedPattern.md
1 parent 9847f9f commit 331984a

6 files changed

Lines changed: 285 additions & 7 deletions

File tree

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

‎docs/Migration-IsolatedStorageDirectoryQualifiedPattern.md‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — `GetFileNames`/`GetDirectoryNames` honour a directory-qualified pattern (ticket #2209)
5+
6+
*2026-08-18.* `IsolatedStorageFile::GetFileNames("sub/*")` now lists `sub`'s files. It used to
7+
return nothing, because both doors iterated the store root and matched the **whole** pattern
8+
against a bare filename.
9+
10+
This is a **widening** — every pattern that worked before returns the same answer — with one
11+
narrowing that closes a hole rather than opening one (§3). Landed under
12+
`docs/StandingApprovals.md` SA-5. No signature, layout or `noexcept` change.
13+
14+
---
15+
16+
## 1. What changed
17+
18+
| Call | Was | Is |
19+
|---|---|---|
20+
| `GetFileNames("sub/*")` | `[]` | `["nested.dat", "other.txt"]` |
21+
| `GetFileNames("sub/*.dat")` | `[]` | `["nested.dat"]` |
22+
| `GetDirectoryNames("sub/*")` | `[]` | `["deeper"]` |
23+
| `GetFileNames("sub/")` | `[]` | everything in `sub` |
24+
| `GetFileNames("nosuchdir/*")` | `[]` | `[]` (unchanged — absent is empty, not an error) |
25+
| `GetFileNames("*")`, `"one*"`, … | — | **unchanged** |
26+
| `GetFileNames("../*")` | `[]` | `ArgumentException` — §3 |
27+
28+
**Results are bare names, not sub-paths.** `GetFileNames("sub/*.dat")` returns `"nested.dat"`, not
29+
`"sub/nested.dat"`. That is .NET's contract, and it is deliberate: `IsolatedStorageFile.cs:177`
30+
maps each hit through `Path.GetFileName` precisely because the store exists to hide its own root.
31+
32+
## 2. The reference
33+
34+
.NET's own source states the contract in a comment above each method:
35+
36+
```csharp
37+
// foo\abc*.txt will give all abc*.txt files in foo directory
38+
public string[] GetFileNames(string searchPattern) { … }
39+
40+
// foo\data* will give all directory names in foo directory that starts with data
41+
public string[] GetDirectoryNames(string searchPattern) { … }
42+
```
43+
44+
Both delegate to `Directory.EnumerateFiles(RootDirectory, searchPattern)`, and
45+
`FileSystemEnumerableFactory.NormalizeInputs` does the splitting:
46+
47+
```csharp
48+
ReadOnlySpan<char> directoryName = Path.GetDirectoryName(expression.AsSpan());
49+
if (directoryName.Length != 0)
50+
{
51+
directory = Path.Join(directory.AsSpan(), directoryName);
52+
expression = expression.Substring(directoryName.Length + 1);
53+
}
54+
```
55+
*(`FileSystemEnumerableFactory.cs:45-56`.)*
56+
57+
The split is at the **last** separator, and a trailing separator leaves an empty expression that
58+
becomes `"*"` — `NormalizeInputs`' own comment says *"We also allowed for expression to be `foo\`
59+
which would translate to `foo\*`"*.
60+
61+
## 3. One deliberate narrowing, on a security boundary
62+
63+
**.NET does not confine the search pattern.** `GetFileNames` and `GetDirectoryNames` are the only
64+
two doors on `IsolatedStorageFile` that bypass `GetFullPath`, so in .NET
65+
`GetFileNames("../*")` escapes the store and lists its parent directory.
66+
67+
This port resolves the directory half through the same `fullPath()` every other door uses, so it
68+
raises `ArgumentException`. Reproducing .NET here would mean **opening a confinement hole to match
69+
a reference that has one**. The port is more restrictive, which is the direction SA-8 does not
70+
reach, and a test pins the asymmetry so it stays a decision rather than becoming an accident.
71+
72+
Containment is about where the path **lands**, not which characters it contains, so
73+
`GetFileNames("a/../a/*")` is fine.
74+
75+
## 4. A second, older divergence this ticket records rather than introduces
76+
77+
.NET rejects a **rooted** pattern outright:
78+
79+
```csharp
80+
if (Path.IsPathRooted(expression))
81+
throw new ArgumentException(SR.Arg_Path2IsRooted, nameof(expression));
82+
```
83+
*(`FileSystemEnumerableFactory.cs:29-30`.)*
84+
85+
This port's `fullPath()` strips leading separators at **every** door, so `"/x"` has always meant
86+
*x relative to the store*. `GetFileNames("/*")` therefore lists the root and `GetFileNames("/etc/*")`
87+
looks for `<store>/etc`. Rejecting a rooted pattern only here would make the type inconsistent
88+
with itself, which is worse than a divergence that is at least uniform. Both rows are pinned.
89+
90+
## 5. To migrate
91+
92+
Nothing to change. If you previously worked around the defect by enumerating a subdirectory
93+
yourself, the pattern form now works:
94+
95+
```cpp
96+
// workaround, still correct
97+
for (const auto& name : store.GetDirectoryNames("*")) { /* … */ }
98+
99+
// now available
100+
const auto saves = store.GetFileNames("saves/*.sav"); // bare names
101+
```
102+
103+
## 6. Evidence
104+
105+
| Mutation | Caught |
106+
|---|---|
107+
| Never split — glob the whole pattern against the root (the pre-#2209 code) | ✅ (2 tests) |
108+
| Split at the **first** separator instead of the last | ✅ |
109+
| Return the sub-path instead of the bare name | ✅ (2 tests) |
110+
| Skip the confinement check on the directory half | ✅ |
111+
| A trailing separator no longer means `"*"` | ✅ |
112+
113+
## 7. Downstream, measured
114+
115+
Per SA-2 condition 5: `mobile-eggbert` uses `IsolatedStorageFile` in one file
116+
(`src/WindowsPhoneSpeedyBlupi/Worlds.cpp`) and calls only `GetUserStoreForApplication`; it never
117+
calls either enumeration door. `cna` has its own
118+
`Microsoft::Xna::Framework::Storage::StorageContainer::GetFileNames`/`GetDirectoryNames`, which
119+
this change does not touch. **Zero affected sites in both.** Neither repository was modified.

‎modules/io-isolated-storage/include/System/IO/IsolatedStorage/IsolatedStorageFile.hpp‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,27 @@ namespace System::IO::IsolatedStorage
6767
[[nodiscard]] std::filesystem::path fullPath(const std::string& relativePath,
6868
const char* paramName) const;
6969

70+
/**
71+
* @brief Splits a search pattern into the directory to enumerate and the glob to match.
72+
*
73+
* Ticket #2209. `GetFileNames("sub/" "*")` must list `sub`'s files, matching .NET, whose two
74+
* enumeration doors delegate to `Directory.EnumerateFiles(RootDirectory, searchPattern)`
75+
* and whose `FileSystemEnumerableFactory.NormalizeInputs` splits the pattern at its last
76+
* separator (`FileSystemEnumerableFactory.cs:45-56`).
77+
*
78+
* The directory half goes through fullPath(), so it stays inside the store. .NET's does
79+
* **not** — see the implementation comment for why this port is deliberately the more
80+
* restrictive of the two.
81+
*
82+
* @param searchPattern The caller's pattern.
83+
* @param glob Receives the final segment, or `"*"` when the pattern ends in a
84+
* separator or is empty.
85+
* @return The directory to enumerate.
86+
* @throws System::ArgumentException if the directory half leaves the store.
87+
*/
88+
[[nodiscard]] std::filesystem::path resolveSearchScope(const std::string& searchPattern,
89+
std::string& glob) const;
90+
7091
/** @throws System::ObjectDisposedException if this store has been Close()d/Remove()d/Dispose()d. */
7192
void throwIfDisposed() const;
7293

‎modules/io-isolated-storage/src/System/IO/IsolatedStorage/IsolatedStorageFile.cpp‎

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,20 +237,80 @@ namespace System::IO::IsolatedStorage
237237
throw IsolatedStorageException("Failed to move isolated storage file: " + src);
238238
}
239239

240+
241+
// -----------------------------------------------------------------------------------------
242+
// Ticket #2209 (2026-08-18): a DIRECTORY-QUALIFIED search pattern.
243+
//
244+
// Both enumeration doors used to iterate rootDirectory_ and glob the whole pattern against a
245+
// bare filename, so GetFileNames("sub/*") returned nothing although sub/nested.dat existed.
246+
// .NET's own source states the contract in a comment above each method:
247+
//
248+
// // foo\abc*.txt will give all abc*.txt files in foo directory
249+
// // foo\data* will give all directory names in foo directory that starts with data
250+
//
251+
// and implements it by delegating to Directory.EnumerateFiles(RootDirectory, searchPattern),
252+
// whose FileSystemEnumerableFactory.NormalizeInputs splits the pattern at its last separator,
253+
// joins the directory half onto the root, and matches only the final segment
254+
// (FileSystemEnumerableFactory.cs:45-56). The RESULT is still a bare name, not a sub-path:
255+
// .NET maps each hit through Path.GetFileName (IsolatedStorageFile.cs:177) precisely because
256+
// the store hides its own root.
257+
//
258+
// ONE DELIBERATE DEVIATION, and it is a narrowing on a security boundary. .NET does NOT run
259+
// the pattern through its containment helper -- GetFileNames/GetDirectoryNames are the only
260+
// two doors on the type that bypass GetFullPath -- so in .NET, GetFileNames("../*") escapes
261+
// the store and lists its parent. This port resolves the directory half through the same
262+
// fullPath() every other door uses, so it is rejected. Reproducing the escape would mean
263+
// introducing a confinement hole to match a reference that has one; the port is more
264+
// RESTRICTIVE here, which SA-8 does not reach, and the asymmetry is pinned by a test.
265+
//
266+
// "" and "sub/" both mean "everything in that directory", matching NormalizeInputs' own
267+
// "We also allowed for expression to be \"foo\\\" which would translate to \"foo\\*\"".
268+
std::filesystem::path IsolatedStorageFile::resolveSearchScope(const std::string& searchPattern,
269+
std::string& glob) const
270+
{
271+
if (searchPattern.find('\0') != std::string::npos)
272+
throw System::ArgumentException("Path must not contain an embedded NUL character.",
273+
"searchPattern");
274+
275+
std::size_t split = std::string::npos;
276+
for (std::size_t i = 0; i < searchPattern.size(); ++i)
277+
if (isDirectorySeparator(searchPattern[i])) split = i;
278+
279+
if (split == std::string::npos) {
280+
glob = searchPattern.empty() ? "*" : searchPattern;
281+
return rootDirectory_;
282+
}
283+
284+
glob = searchPattern.substr(split + 1);
285+
if (glob.empty()) glob = "*";
286+
287+
const std::string directoryPart = searchPattern.substr(0, split);
288+
// A pattern that is nothing but separators names the root itself, which fullPath()
289+
// rejects as empty -- correctly for every other door, and wrongly for this one.
290+
std::size_t firstReal = 0;
291+
while (firstReal < directoryPart.size() && isDirectorySeparator(directoryPart[firstReal]))
292+
++firstReal;
293+
if (firstReal == directoryPart.size()) return rootDirectory_;
294+
295+
return fullPath(directoryPart, "searchPattern");
296+
}
297+
240298
std::vector<std::string> IsolatedStorageFile::GetFileNames(const std::string& searchPattern) const
241299
{
242300
throwIfDisposed();
243301
std::vector<std::string> names;
244-
if (!std::filesystem::exists(rootDirectory_)) return names;
302+
std::string glob;
303+
const std::filesystem::path scope = resolveSearchScope(searchPattern, glob);
304+
if (!std::filesystem::exists(scope)) return names;
245305
std::error_code ec;
246-
std::filesystem::directory_iterator it(rootDirectory_, ec);
306+
std::filesystem::directory_iterator it(scope, ec);
247307
if (ec)
248308
throw IsolatedStorageException(
249309
"Failed to enumerate isolated storage files (" + ec.message() + ")");
250310
for (const auto& entry : it) {
251311
if (!entry.is_regular_file(ec) || ec) { ec.clear(); continue; }
252312
std::string name = entry.path().filename().string();
253-
if (globMatch(searchPattern, name))
313+
if (globMatch(glob, name))
254314
names.push_back(name);
255315
}
256316
std::sort(names.begin(), names.end());
@@ -305,16 +365,18 @@ namespace System::IO::IsolatedStorage
305365
{
306366
throwIfDisposed();
307367
std::vector<std::string> names;
308-
if (!std::filesystem::exists(rootDirectory_)) return names;
368+
std::string glob;
369+
const std::filesystem::path scope = resolveSearchScope(searchPattern, glob);
370+
if (!std::filesystem::exists(scope)) return names;
309371
std::error_code ec;
310-
std::filesystem::directory_iterator it(rootDirectory_, ec);
372+
std::filesystem::directory_iterator it(scope, ec);
311373
if (ec)
312374
throw IsolatedStorageException(
313375
"Failed to enumerate isolated storage directories (" + ec.message() + ")");
314376
for (const auto& entry : it) {
315377
if (!entry.is_directory(ec) || ec) { ec.clear(); continue; }
316378
std::string name = entry.path().filename().string();
317-
if (globMatch(searchPattern, name))
379+
if (globMatch(glob, name))
318380
names.push_back(name);
319381
}
320382
std::sort(names.begin(), names.end());

‎modules/io-isolated-storage/tests/System/IO/IsolatedStorage/IsolatedStorageConfinementTests.cpp‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -835,6 +835,82 @@ namespace {
835835
EXPECT_GE(s.getUsedSizeProperty(), 0); // recurses into a_dir without throwing
836836
}
837837

838+
// =====================================================================================
839+
// Ticket #2209 -- a directory-qualified search pattern.
840+
// =====================================================================================
841+
842+
TEST_F(IsolatedStorageConfinementTest, Fix2209_ADirectoryQualifiedPatternEnumeratesThatDirectory)
843+
{
844+
// Before #2209 both doors iterated the root and globbed the WHOLE pattern against a bare
845+
// filename, so this returned nothing although the file exists. .NET's own source states
846+
// the contract in a comment above each method -- "foo\\abc*.txt will give all abc*.txt
847+
// files in foo directory" -- and implements it through
848+
// FileSystemEnumerableFactory.NormalizeInputs (FileSystemEnumerableFactory.cs:45-56).
849+
auto s = store();
850+
s.CreateDirectory("sub");
851+
s.CreateDirectory("sub/deeper");
852+
{ auto st = s.CreateFile("sub/nested.dat"); st.Close(); }
853+
{ auto st = s.CreateFile("sub/other.txt"); st.Close(); }
854+
{ auto st = s.CreateFile("root.dat"); st.Close(); }
855+
856+
EXPECT_EQ(s.GetFileNames("sub/*").size(), 2u);
857+
EXPECT_EQ(s.GetFileNames("sub/*.dat"), std::vector<std::string>{"nested.dat"});
858+
EXPECT_EQ(s.GetDirectoryNames("sub/*"), std::vector<std::string>{"deeper"});
859+
860+
// THE RESULT IS A BARE NAME, not a sub-path. .NET maps each hit through
861+
// Path.GetFileName (IsolatedStorageFile.cs:177) precisely because the store hides its
862+
// own root, so returning "sub/nested.dat" would leak a layout the type exists to hide.
863+
const auto names = s.GetFileNames("sub/*.dat");
864+
ASSERT_EQ(names.size(), 1u);
865+
EXPECT_EQ(names[0].find('/'), std::string::npos);
866+
867+
// A trailing separator means "everything in that directory" --
868+
// NormalizeInputs' own "we also allowed for expression to be \"foo\\\"".
869+
EXPECT_EQ(s.GetFileNames("sub/").size(), 2u);
870+
871+
// An unqualified pattern is unchanged, and still does not descend.
872+
EXPECT_EQ(s.GetFileNames("*"), std::vector<std::string>{"root.dat"});
873+
EXPECT_EQ(s.GetDirectoryNames("*"), std::vector<std::string>{"sub"});
874+
875+
// A directory that does not exist is empty, not an error -- the same answer the root
876+
// gives when the store has just been created.
877+
EXPECT_TRUE(s.GetFileNames("nosuchdir/*").empty());
878+
}
879+
880+
TEST_F(IsolatedStorageConfinementTest, Fix2209_TheDirectoryHalfIsConfinedAndDotNetsIsNot)
881+
{
882+
// A DELIBERATE NARROWING, pinned so it is a decision rather than an accident. .NET does
883+
// not run the search pattern through its containment helper: GetFileNames and
884+
// GetDirectoryNames are the only two doors on IsolatedStorageFile that bypass
885+
// GetFullPath, so in .NET `GetFileNames("../*")` escapes the store and lists its parent.
886+
// This port resolves the directory half through the same fullPath() every other door
887+
// uses. Reproducing .NET here would mean opening a confinement hole to match a reference
888+
// that has one.
889+
auto s = store();
890+
{ auto st = s.CreateFile("inside.dat"); st.Close(); }
891+
892+
for (const char* escaping : {"../*", "sub/../../*", "../"}) {
893+
EXPECT_THROW((void)s.GetFileNames(escaping), System::ArgumentException) << escaping;
894+
EXPECT_THROW((void)s.GetDirectoryNames(escaping), System::ArgumentException) << escaping;
895+
}
896+
897+
// A LEADING SEPARATOR IS NOT AN ESCAPE HERE, and that is a second, older divergence this
898+
// test records rather than introduces. .NET rejects a rooted search pattern outright --
899+
// NormalizeInputs opens with `if (Path.IsPathRooted(expression)) throw new
900+
// ArgumentException(SR.Arg_Path2IsRooted, ...)` (FileSystemEnumerableFactory.cs:29-30).
901+
// This port's fullPath() strips leading separators at EVERY door, so "/x" has always
902+
// meant "x relative to the store". Rejecting it only in the pattern would make the type
903+
// inconsistent with itself, which is worse than a divergence that is uniform.
904+
EXPECT_EQ(s.GetFileNames("/*"), std::vector<std::string>{"inside.dat"});
905+
EXPECT_TRUE(s.GetFileNames("/etc/*").empty()); // root/etc, which does not exist
906+
907+
// ...and a harmless ".." that stays inside is fine, because containment is about where
908+
// the path LANDS, not about which characters it contains.
909+
s.CreateDirectory("a");
910+
{ auto st = s.CreateFile("a/deep.dat"); st.Close(); }
911+
EXPECT_EQ(s.GetFileNames("a/../a/*"), std::vector<std::string>{"deep.dat"});
912+
}
913+
838914
// =====================================================================================
839915
// The residual this repair deliberately does not close (#2208).
840916
// =====================================================================================

‎plan.sqlite3‎

0 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)