Skip to content

Commit 833b04c

Browse files
robertvokacrobertvokac
committed
fix(core): honour XDG bases and SpecialFolderOption on POSIX (#2320, SR-AUD-105)
Decision 1 (XDG), answer (b): XDG_CONFIG_HOME and XDG_DATA_HOME are honoured when set AND absolute. The reference's test is transcribed rather than paraphrased -- GetFolderPathCore.Unix.cs:157-161 asks 'is it absolute', not 'is it set', so an EMPTY value falls back exactly as an unset one does. A naive getenv() != nullptr implementation fails that row, and a mutation pins it. Decision 2 turned out NOT to be a user question: the reference answers it, so it was derived. DoNotVerify returns the path unchecked; None -- the default -- runs access(R_OK) and returns "" on failure; Create creates every missing component; an undefined option raises ArgumentOutOfRangeException while an undefined folder deliberately does not. THAT DEFAULT IS A BEHAVIOUR CHANGE. All three options previously returned the path, so a caller could not tell a real directory from a name. GetFolderPath(Desktop) is now "" on a machine with no ~/Desktop. ::mkdir per component rather than System::IO::Directory, on purpose: Core.Base does not depend on modules/io, and taking that edge to create one directory would be a far larger change than the behaviour it buys. A TEST DEFECT HAD TO BE FIXED ON THE WAY. Five rows asserted a non-empty result and passed only because this machine has ~/Desktop and ~/.config; once the default verifies, they encode the environment rather than the contract, which SA-6 calls a defect in the test. One further row asserted the OPPOSITE of the new contract and is rewritten to state the real one. THE PREMISE FOR DECLINING FULL XDG WAS WRONG AND IS CORRECTED RATHER THAN LEFT STANDING. The user was told option (c) would 'cross from parity into invention'. Measured, every remaining XDG behaviour has a .NET mapping, because ReadXdgDirectory reads user-dirs.dirs. The reason to stop here is scope, not invention, so eight further diverging rows -- including Personal returning $HOME where .NET returns a Documents subdirectory -- are filed as #2364 for a decision on accurate information. Five mutations, all caught. Gate 17,278 run, 0 failed, 38 executables.
1 parent 00786fc commit 833b04c

7 files changed

Lines changed: 369 additions & 31 deletions

File tree

‎CLAUDE.md‎

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

‎audit/AUDIT_FINDINGS_INDEX.md‎

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

‎docs/Migration-EnvironmentXdgAndFolderOption.md‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — XDG base directories, and `SpecialFolderOption` on POSIX (ticket #2320)
5+
6+
*2026-08-18.* `Environment::GetFolderPath` ignored `XDG_CONFIG_HOME` and `XDG_DATA_HOME`, and
7+
accepted `SpecialFolderOption` on POSIX without honouring it. Both are fixed.
8+
9+
Landed under `docs/StandingApprovals.md` SA-5, on the user's decision of the same date (SA-11).
10+
**The default option's behaviour changes** — read §2. No signature, layout, vtable or `noexcept`
11+
change.
12+
13+
---
14+
15+
## 1. XDG — decision 1, answer (b)
16+
17+
| `XDG_CONFIG_HOME` | `ApplicationData` was | is |
18+
|---|---|---|
19+
| unset | `$HOME/.config` | `$HOME/.config` |
20+
| `/srv/cfg` (absolute) | `$HOME/.config` | **`/srv/cfg`** |
21+
| `relative/cfg` | `$HOME/.config` | `$HOME/.config` |
22+
| `""` (empty) | `$HOME/.config` | `$HOME/.config` |
23+
24+
`XDG_DATA_HOME` behaves identically against `LocalApplicationData` / `$HOME/.local/share`.
25+
26+
The test is `Environment.GetFolderPathCore.Unix.cs:157-161` transcribed —
27+
`config is null || !config.StartsWith('/')` — so it is **"is it absolute"**, not "is it set". An
28+
empty value falls back exactly as an unset one does, because an empty string does not start with
29+
`/` either. The XDG specification agrees: a relative value *"must be ignored"*. A naive
30+
`getenv() != nullptr` implementation fails that row, and a test pins it.
31+
32+
## 2. `SpecialFolderOption` is now honoured on POSIX — decision 2
33+
34+
This half was **not** a user question in the end: .NET answers it itself
35+
(`Environment.GetFolderPathCore.Unix.cs:26-47`), so it is a derivation under SA-5.
36+
37+
| Option | Behaviour |
38+
|---|---|
39+
| `DoNotVerify` | return the path unchecked |
40+
| `None` — **the default** | `access(path, R_OK)`; **return `""` if that fails** |
41+
| `Create` | verify; on failure create every missing component, then return the path |
42+
| undefined value | `ArgumentOutOfRangeException`, paramName `option` (`Environment.cs:149-163`) |
43+
44+
**Before this ticket all three returned the path**, so a caller could not tell a real directory
45+
from a name. The behaviour change that matters:
46+
47+
```cpp
48+
// A machine with no ~/Desktop:
49+
Environment::GetFolderPath(SpecialFolder::Desktop); // was "/home/u/Desktop", is now ""
50+
```
51+
52+
If you want the old unconditional answer, ask for it: pass
53+
`SpecialFolderOption::DoNotVerify`. If you want the directory to exist, pass
54+
`SpecialFolderOption::Create`.
55+
56+
`Directory::CreateDirectory` is deliberately **not** used — `Core.Base` does not depend on
57+
`modules/io`, and taking that edge to create one directory would be a far larger change than the
58+
behaviour it buys. `::mkdir` per component is the whole implementation.
59+
60+
Note the option was already meaningful on Windows, where its values *are* the CSIDL flags and are
61+
OR-ed into the folder id. This change makes POSIX agree with a contract Windows always had.
62+
63+
## 3. What this ticket deliberately did NOT change — and a premise it corrected
64+
65+
Option (c), *full XDG*, was declined **on a premise that turned out to be wrong**, and this is
66+
recorded rather than quietly acted on. The decision was offered with the reasoning *"part of it
67+
has no .NET mapping, so it would cross from parity into invention"*. Measured against the
68+
reference, **every remaining XDG behaviour does have a .NET mapping**: `ReadXdgDirectory`
69+
(`Environment.GetFolderPathCore.Unix.cs:165+`) reads `user-dirs.dirs` out of the XDG config
70+
directory and backs `Desktop`, `MyDocuments`, `MyMusic`, `MyPictures`, `MyVideos` and `Templates`.
71+
72+
So the reason to stop here is *scope*, not *invention*. Eight further rows diverge from the
73+
reference and are filed as **ticket #2364** for a decision made on accurate information:
74+
75+
| `SpecialFolder` | this port | .NET on Linux |
76+
|---|---|---|
77+
| `Personal` / `MyDocuments` | `$HOME` | `ReadXdgDirectory(XDG_DOCUMENTS_DIR, "Documents")` |
78+
| `Desktop` / `DesktopDirectory` | `$HOME/Desktop` | `ReadXdgDirectory(XDG_DESKTOP_DIR, …)` |
79+
| `MyMusic` / `MyPictures` / `MyVideos` | `$HOME/Music` … | `ReadXdgDirectory(…)` |
80+
| `Templates` | `$HOME/Templates` | `ReadXdgDirectory(XDG_TEMPLATES_DIR, …)` |
81+
| `CommonApplicationData` | `/etc` | **`/usr/share`** |
82+
| `Fonts` | `/usr/share/fonts` | **`$HOME/.fonts`** |
83+
| `ProgramFiles`, `System` | `/usr`, `/usr/lib` | **not mapped** (empty) |
84+
| `CommonTemplates` | not mapped | `/usr/share/templates` |
85+
86+
One row **was** adopted here because it is a safety property rather than a path preference: when
87+
`HOME` is unset, the reference falls back to `/` and says why — `/` is not writable by a non-root
88+
user, so an application cannot silently write private data into a path built from an empty
89+
string.
90+
91+
## 4. A test defect this ticket had to fix on the way
92+
93+
Five existing rows asserted `GetFolderPath(...)` was non-empty, and passed only because the
94+
development machine happens to have `~/Desktop` and `~/.config`. Once the default option
95+
verifies, they encode the environment rather than the contract — exactly what
96+
`docs/StandingApprovals.md` SA-6 calls a defect in the test. They now pass `DoNotVerify`, which is
97+
what they were always about.
98+
99+
One row asserted the **opposite** of the new contract (`DoNotVerify` equals the default) and is
100+
rewritten to state the real one: the two agree exactly when the directory is readable, and differ
101+
otherwise — which is the entire point of the option.

‎modules/core/include/System/Environment.hpp‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,12 @@ class Environment {
108108
* @brief Specifies options to use when getting the path to a system special folder.
109109
*
110110
* C++ counterpart of .NET System.Environment.SpecialFolderOption.
111-
* Values correspond to Windows CSIDL flags and are meaningful only on Windows;
112-
* on POSIX they are accepted for API compatibility and ignored.
111+
*
112+
* The values are the Windows CSIDL flags, and on Windows they are OR-ed into the folder id.
113+
* **On POSIX they are honoured too, since ticket #2320** -- they used to be accepted and
114+
* ignored. `DoNotVerify` returns the path unchecked, `None` (the default) returns `""` when
115+
* the directory is not readable, and `Create` creates it. Transcribed from
116+
* `Environment.GetFolderPathCore.Unix.cs:26-47`.
113117
*/
114118
enum class SpecialFolderOption {
115119
/** @brief No option is specified. */
@@ -325,9 +329,7 @@ class Environment {
325329
* C++ counterpart of .NET Environment.GetFolderPath(SpecialFolder, SpecialFolderOption).
326330
* The option is accepted for API compatibility but ignored on POSIX.
327331
*/
328-
[[nodiscard]] static std::string GetFolderPath(SpecialFolder folder, SpecialFolderOption) {
329-
return GetFolderPath(folder);
330-
}
332+
[[nodiscard]] static std::string GetFolderPath(SpecialFolder folder, SpecialFolderOption option);
331333

332334
/**
333335
* @brief Gets the path to the system directory.

‎modules/core/src/System/Environment.cpp‎

Lines changed: 93 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "System/Environment.hpp"
55
#ifdef _WIN32
66
#include "System/ApplicationException.hpp"
7+
#include "System/ArgumentOutOfRangeException.hpp"
78
#endif
89
#include "System/IO/DirectoryNotFoundException.hpp"
910

@@ -24,9 +25,11 @@
2425
#elif defined(__EMSCRIPTEN__)
2526
# include <unistd.h>
2627
# include <climits>
28+
# include <sys/stat.h> // GetFolderPath's SpecialFolderOption::Create (#2320)
2729
#else
2830
# include <unistd.h>
2931
# include <climits>
32+
# include <sys/stat.h> // GetFolderPath's SpecialFolderOption::Create (#2320)
3033
# include <pwd.h>
3134
# include <time.h>
3235
# include <sys/resource.h>
@@ -319,32 +322,108 @@ std::string Environment::ExpandEnvironmentVariables(const std::string& name) {
319322
return result;
320323
}
321324

325+
namespace {
326+
327+
#if !defined(_WIN32)
328+
/// The XDG base-directory rule, transcribed from `Environment.GetFolderPathCore.Unix.cs:153-163`.
329+
///
330+
/// The test is `config is null || !config.StartsWith('/')` -- so a variable is honoured ONLY when
331+
/// it is set AND ABSOLUTE, and an empty value falls back exactly as an unset one does, because an
332+
/// empty string does not start with '/' either. The XDG specification says the same: a relative
333+
/// value "must be ignored". Ticket #2320 decision 1, answer (b).
334+
std::string xdgBase(const char* variable, const std::string& home, const char* fallback) {
335+
const char* value = std::getenv(variable);
336+
if (value != nullptr && value[0] == '/') return std::string(value);
337+
return home + fallback;
338+
}
339+
340+
/// `Interop.Sys.Access(path, R_OK) == 0` -- the verification .NET's DEFAULT option performs.
341+
bool readable(const std::string& path) {
342+
return !path.empty() && ::access(path.c_str(), R_OK) == 0;
343+
}
344+
345+
/// `Directory.CreateDirectory(path)`: creates every missing component, like `mkdir -p`.
346+
///
347+
/// Written with `::mkdir` rather than `System::IO::Directory` on purpose -- `Core.Base` does not
348+
/// depend on `modules/io`, and giving it that edge to create one directory would be a far larger
349+
/// change than the behaviour it buys.
350+
void createDirectoryTree(const std::string& path) {
351+
if (path.empty()) return;
352+
std::string partial;
353+
partial.reserve(path.size());
354+
for (std::size_t i = 0; i < path.size(); ++i) {
355+
partial.push_back(path[i]);
356+
const bool last = (i + 1 == path.size());
357+
if (path[i] == '/' || last) {
358+
if (partial != "/" ) ::mkdir(partial.c_str(), 0777); // EEXIST is the normal case
359+
}
360+
}
361+
}
362+
#endif
363+
364+
} // namespace
365+
322366
std::string Environment::GetFolderPath(SpecialFolder folder) {
367+
return GetFolderPath(folder, SpecialFolderOption::None);
368+
}
369+
370+
std::string Environment::GetFolderPath(SpecialFolder folder, SpecialFolderOption option) {
371+
// `Environment.cs:149-163`: the OPTION is validated even though the FOLDER is not -- an
372+
// undefined folder legitimately returns "" (see #2321), an undefined option never does.
373+
if (option != SpecialFolderOption::None && option != SpecialFolderOption::Create &&
374+
option != SpecialFolderOption::DoNotVerify) {
375+
throw System::ArgumentOutOfRangeException(
376+
"option", "Illegal enum value: " + std::to_string(static_cast<long long>(option)) + ".");
377+
}
378+
323379
#if defined(_WIN32)
380+
// Windows resolves and applies the flags in one call: SpecialFolderOption's values ARE the
381+
// CSIDL flags, so they are simply OR-ed into the folder id, which is why this branch does not
382+
// repeat the POSIX verification below.
324383
char buf[MAX_PATH];
325-
if (SHGetFolderPathA(nullptr, static_cast<int>(folder), nullptr, SHGFP_TYPE_CURRENT, buf) == S_OK)
384+
const int csidl = static_cast<int>(folder) | static_cast<int>(option);
385+
if (SHGetFolderPathA(nullptr, csidl, nullptr, SHGFP_TYPE_CURRENT, buf) == S_OK)
326386
return std::string(buf);
327387
return "";
328388
#else
329389
const char* home = std::getenv("HOME");
330-
std::string h = home ? std::string(home) : std::string();
390+
// `GetFolderPathCore.Unix.cs:76-81`: fall back to "/" when the home directory is unknown.
391+
// .NET states the reason and it is a safety property rather than a tidy default -- "/" is not
392+
// writable by a non-root user, so an application cannot silently write private data into a
393+
// path built from an empty string.
394+
std::string h = (home != nullptr && home[0] != '\0') ? std::string(home) : std::string("/");
395+
if (h == "/") h.clear(); // so h + "/.config" stays "/.config" rather than "//.config"
396+
397+
std::string path;
331398
switch (folder) {
332399
case SpecialFolder::Personal: // == MyDocuments (0x0005), returns home
333-
case SpecialFolder::UserProfile: return h;
400+
case SpecialFolder::UserProfile: path = h.empty() ? "/" : h; break;
334401
case SpecialFolder::Desktop:
335-
case SpecialFolder::DesktopDirectory: return h + "/Desktop";
336-
case SpecialFolder::MyMusic: return h + "/Music";
337-
case SpecialFolder::MyPictures: return h + "/Pictures";
338-
case SpecialFolder::MyVideos: return h + "/Videos";
339-
case SpecialFolder::ApplicationData: return h + "/.config";
340-
case SpecialFolder::LocalApplicationData: return h + "/.local/share";
341-
case SpecialFolder::CommonApplicationData: return "/etc";
342-
case SpecialFolder::ProgramFiles: return "/usr";
343-
case SpecialFolder::System: return "/usr/lib";
344-
case SpecialFolder::Fonts: return "/usr/share/fonts";
345-
case SpecialFolder::Templates: return h + "/Templates";
402+
case SpecialFolder::DesktopDirectory: path = h + "/Desktop"; break;
403+
case SpecialFolder::MyMusic: path = h + "/Music"; break;
404+
case SpecialFolder::MyPictures: path = h + "/Pictures"; break;
405+
case SpecialFolder::MyVideos: path = h + "/Videos"; break;
406+
// The two XDG bases, and the ONLY two this ticket adopts -- see the migration note for
407+
// the rest of the table, which diverges from the reference in six further places and is
408+
// ticket #2364 rather than a silent widening here.
409+
case SpecialFolder::ApplicationData: path = xdgBase("XDG_CONFIG_HOME", h, "/.config"); break;
410+
case SpecialFolder::LocalApplicationData:
411+
path = xdgBase("XDG_DATA_HOME", h, "/.local/share"); break;
412+
case SpecialFolder::CommonApplicationData: path = "/etc"; break;
413+
case SpecialFolder::ProgramFiles: path = "/usr"; break;
414+
case SpecialFolder::System: path = "/usr/lib"; break;
415+
case SpecialFolder::Fonts: path = "/usr/share/fonts"; break;
416+
case SpecialFolder::Templates: path = h + "/Templates"; break;
346417
default: return "";
347418
}
419+
420+
// `GetFolderPathCore.Unix.cs:26-47`. Note what the DEFAULT does: `None` VERIFIES, and returns
421+
// "" when the directory is not readable. Before #2320 this port returned every path
422+
// unconditionally, so a caller could not tell a real directory from a name.
423+
if (path.empty() || option == SpecialFolderOption::DoNotVerify || readable(path)) return path;
424+
if (option == SpecialFolderOption::None) return "";
425+
createDirectoryTree(path);
426+
return path;
348427
#endif
349428
}
350429

0 commit comments

Comments
 (0)