Fix Issue 13367 - Buffer overflow when setting PATH

[denis-sh]

https://issues.dlang.org/show_bug.cgi?id=13367

Fixup for pull #2332.

[denis-sh]

So yes, I failed to copy-paste-modify code from my allocators to the pull and reviewers failed to see the mistake.

[DmitryOlshansky]

This tells me, aside from the fact that I must be blind, that there has to be more tests.
In particular all string widths and lengths both below and beyond the limit.

For instance this fails for me consistently, with any length:

mport std.internal.cstring;
import core.stdc.string;

alias Seq(T...) = T;

void main()
{
    foreach(i, T; Seq!(string, wstring, dstring))
    {
        T v = "Кукарача";
        foreach(_; 0..2)
            v ~= v;

        auto t = v.tempCString();
        foreach(j; 0..v.length)
            assert(t.ptr[j] == v[j], "bug in "~T.stringof);
    }
}

UPDATE: fixed a typo, LOL.

[denis-sh]

For instance this fails for me consistently, with any length:

The test is incorrect. It should call v.tempCString!C() where C is typeof(cast() T.init[0]).

[DmitryOlshansky]

@denis-sh That's better. Now the following hits access violation with old version. I guess it would work with your patch.

import std.internal.cstring;
import core.stdc.string;

alias Seq(T...) = T;

void main(){
    foreach(i, T; Seq!(string, wstring, dstring))
    {
        T v = "Кукорача";
        foreach(_; 0..10)
            v ~= v;

        auto t = v.tempCString!(typeof(cast()T.init[0]));
        foreach(j; 0..v.length)
            assert(t.ptr[j] == v[j], "bug in "~T.stringof);
    }
}

[DmitryOlshansky]

Wouldn't it make more sense to have default character type to be that of passed string unless explicitly provided?

[denis-sh]

Wouldn't it make more sense to have default character type to be that of passed string unless explicitly provided?

C function is generally not templated, so API is designed with this case in mind:

void setEnvironment(C)(in C[] name, in C[] value)
if(isSomeChar!C)
{ enforce(setenv(name.tempCString(), value.tempCString(), 1) != -1); }

[DmitryOlshansky]

Makes sense. Then please integrate tests for this bug and it should be ready to go.

[denis-sh]

Added unittest for the issue.

[DmitryOlshansky]

Auto-merge toggled on

[DmitryOlshansky]

Thanks!