fix Issue 23215: segfault in std.file.remove

[WebFreak001]

string s = null;
remove(s);

would break as s is null and the removeImpl function has two arguments:

1. name as D string
2. name as tempCString

for dynamic ranges the D string is resolved from the tempCString by slicing the pointer up to strlen, however if the actual data is null, then the returned string by tempCString is a null pointer and attempting to strlen the null pointer segfaults.

Now a FileException is thrown, stating that a null string is not allowed and in the other methods that contained this strlen bug, "[null]" is now used as filename in the FileException.

[dlang-bot]

Thanks for your pull request, @WebFreak001!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	23215	major	calling std.file.remove with null string segfaults in strlen

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#8483"

[CyberShadow]

IMO this is much more complicated than it should be. Doing this for all file APIs would be impractical.

I suggest fix this with the least possible code by doing in removeImpl the same as what's done in cenforce:

-             auto len = strlen(namez);
+             auto len = namez ? strlen(namez) : 0;

[WebFreak001]

that would mean the error messages will say : message though, imo [null]: message would be a lot less confusing to users

[CyberShadow]

that would mean the error messages will say : message though

For that, we should quote it somehow anyway, as special characters will mess up the message in a similar way. It also should be done consistently for all error messages in std.file. If that's something that we want to do, I suggest doing that in a separate PR, and just leave the segfault fix here.

I've been using str.only.format!"%(%s%)" in my code to quote strings, but I haven't checked if std.file's imports/dependencies are set up for that.

[CyberShadow]

[null]

Also, not to start a bikeshed war, but in D that syntax means an array with one null element. We shouldn't use that syntax to indicate a null string.

[WebFreak001]

I have changed back the [null] changes and made the strlen check for null.

However I still think the exception improvement is valuable, so I moved the logic into the exception constructor and named it (null)

[CyberShadow]

Thanks, LGTM though I'm not a big fan of showing empty non-null strings as null, we should avoid computers gaslighting users where possible :)

[WebFreak001]

squashed commits, made it only show (null) for actually null strings.

[dkorpel]

This assert fails currently

[WebFreak001]

on linux it resulted in Bad address on my system, I adjusted the error messages now and test them on linux and windows. (message is platform-dependent) Otherwise just a check for FileException is used.

If the message is different on other linux distros (such as in the CI), I will also limit it to a prefix check there.