All CI checks are green now (PHP code style, JS/CSS lint, unit tests, integration tests across PHP 8.1-8.5 and MySQL/MariaDB/Postgres, system tests, PHPStan, and the drone build).

@HLeithner @richard67 @muhme @tecpromotion @MacJoom would one of you have a moment to take a look at this? It's a small, targeted fix for the open_basedir warning reported in #46313 - happy to make any changes needed.

Please use the Pull Request-Template.

Additional comments - Idea

I suggest inserting a dog sign (@) before the is_dir() functions.

        if (! @is_dir($path)) {
            $path = JPATH_ROOT . '/' . $path;
        }

In this PR, TRIM is applied 2 times to the same variable.
To the variable $directory 2 times TRIM()

$directory = trim((string) $fieldNode->getAttribute('directory'), '/');
$fieldNode->setAttribute('directory', trim('images/' . $directory, '/'));

Either remove TRIM in the first line or remove TRIM in the second

$directory = trim((string) $fieldNode->getAttribute('directory'), '/');
$fieldNode->setAttribute('directory', 'images/' . $directory);

@korenevskiy Thanks for the review!

Re the double trim: fair catch that it looked redundant. It wasn't strictly a double-trim on the same value (the second trim() was on the concatenated 'images/' . $directory string, needed to strip the trailing slash when $directory is empty), but the intent is clearer written explicitly. Updated to:

$directory = trim((string) $fieldNode->getAttribute('directory'), '/');
$fieldNode->setAttribute('directory', $directory === '' ? 'images' : 'images/' . $directory);

Re the @is_dir() suggestion: that call lives in FilelistField::getOptions(), which isn't touched by this PR. This fix addresses the root cause — the directory attribute was being written as an absolute path (/images/...), so FilelistField treated it as outside JPATH_ROOT and is_dir() tripped the open_basedir restriction. Now that it's always written as a relative path, is_dir() should no longer hit that restriction for this field, so suppressing the warning there isn't needed for this issue. Joomla's coding standards also generally avoid the @ error-suppression operator since it can mask unrelated real errors, so I'd rather not introduce it as a separate change here — happy to discuss further if you think there's still a case it doesn't cover.

Maybe so?

$directory = (string) $fieldNode->getAttribute('directory');
$fieldNode->setAttribute('directory', trim('images/' . $directory, '/'));

Maybe so?

$directory = (string) $fieldNode->getAttribute('directory');
$fieldNode->setAttribute('directory', trim('images/' . $directory, '/'));

If the code works equally well in terms of speed, then we must write in such a way that the code is easy to read, without strain on the brain.

@korenevskiy Good instinct on readability, but dropping the first trim() would actually reintroduce a bug: if the directory attribute has a leading/trailing slash (e.g. /foo/), skipping it gives 'images/' . '/foo/' = 'images//foo/'. The outer trim(..., '/') only strips characters from the very start/end of the string, so it only removes the trailing slash, leaving 'images//foo' — a double slash in the middle of the path. That's exactly the kind of malformed path that caused the original open_basedir issue, so I don't want to risk reintroducing it.

The first trim is what guarantees $directory itself has no stray slashes before we concatenate, so the join is always clean.

That said, I did simplify it in the latest push to make the intent explicit instead of looking like a "double trim":

$directory = trim((string) $fieldNode->getAttribute('directory'), '/');
$fieldNode->setAttribute('directory', $directory === '' ? 'images' : 'images/' . $directory);

This avoids the visual "trim called twice" while still being correct for the empty and slash-padded cases. Let me know if that reads better.

Additional comments - Idea

I suggest inserting a dog sign (@) before the is_dir() functions.

        if (! @is_dir($path)) {
            $path = JPATH_ROOT . '/' . $path;
        }

try to avoid @xxx at all costs.

Additional comments - Idea

I suggest inserting a dog sign (@) before the is_dir() functions.

        if (! @is_dir($path)) {
            $path = JPATH_ROOT . '/' . $path;
        }

try to avoid @XXX at all costs.

Agreed, that's why I left it out. Current fix avoids needing it.

Yes, you're 100% right, the original code was deep and fair. I've been thinking for a long time, with two TRIM it looks easy, but your last option explains the reason.

Let's focus on the last option.

Sounds good, thanks for the discussion!

But why do you use === ", Is that how you write PSR? I wanted to learn about the methodology of the comparison code with an empty string.

$directory = trim((string) $fieldNode->getAttribute('directory'), '/');
$fieldNode->setAttribute('directory', $directory  ? 'images/' . $directory : 'images';

But why do you use === ", Is that how you write PSR? I wanted to learn about the methodology of the comparison code with an empty string. for personal interest.

$directory = trim((string) $fieldNode->getAttribute('directory'), '/');
$fieldNode->setAttribute('directory', $directory  ? 'images/' . $directory : 'images';

Just following Joomla's coding standards, which require strict comparisons (===) over loose truthy checks — $directory ? ... : ... would also treat "0" as empty, which === '' avoids. Not a PSR rule, a Joomla-specific one.