I have discovered an annoyance in using vibe.d instead of another web framework. Simple errors in indexing crash the entire application.

For example:

int[3] arr;
arr[3] = 5;

Compare this to, let's say, a malformed unicode string (exception), malformed JSON data (exception), file not found (exception), etc.

Technically this is a programming error, and a bug. But memory hasn't actually been corrupted. The system properly stopped me from corrupting memory. But my reward is that even though this fiber threw an Error, and I get an error message in the log showing me the bug, the web server itself is now out of commission. No other pages can be served. This is like the equivalent of having a guard rail on a road not only stop you from going off the cliff but proactively disable your car afterwards to prevent you from more harm.

This seems like a large penalty for "almost" corrupting memory. No other web framework I've used crashes the entire web server for such a simple programming error. And vibe.d has no choice. There is no guarantee the stack is properly unwound, so it has to accept the characterization of this is a program-ending error by the D runtime.

I am considering writing a set of array wrappers that throw exceptions when trying to access out of bounds elements. This comes with its own set of problems, but at least the web server should continue to run.

What are your thoughts? Have you run into this? If so, how did you solve it?

-Steve

On Wed, May 31, 2017 at 09:04:52AM -0400, Steven Schveighoffer via Digitalmars-d wrote:
> I have discovered an annoyance in using vibe.d instead of another web framework. Simple errors in indexing crash the entire application.
> 
> For example:
> 
> int[3] arr;
> arr[3] = 5;
> 
> Compare this to, let's say, a malformed unicode string (exception),
> malformed JSON data (exception), file not found (exception), etc.
> 
> Technically this is a programming error, and a bug. But memory hasn't actually been corrupted. The system properly stopped me from corrupting memory. But my reward is that even though this fiber threw an Error, and I get an error message in the log showing me the bug, the web server itself is now out of commission. No other pages can be served. This is like the equivalent of having a guard rail on a road not only stop you from going off the cliff but proactively disable your car afterwards to prevent you from more harm.
[...]

Isn't it customary to have the webserver launched by a script that restarts it whenever it crashes (after logging a message in an emergency logfile)?  Not an ideal solution, I know, but at least it minimizes downtime.

On another note, why didn't the compiler reject the above code? I thought it checks static arrays bounds at compile time whenever possible. Did I remember wrong?


T

-- 
Change is inevitable, except from a vending machine.

On 5/31/17 9:21 AM, H. S. Teoh via Digitalmars-d wrote:
> On Wed, May 31, 2017 at 09:04:52AM -0400, Steven Schveighoffer via Digitalmars-d wrote:
>> I have discovered an annoyance in using vibe.d instead of another web
>> framework. Simple errors in indexing crash the entire application.
>>
>> For example:
>>
>> int[3] arr;
>> arr[3] = 5;
>>
>> Compare this to, let's say, a malformed unicode string (exception),
>> malformed JSON data (exception), file not found (exception), etc.
>>
>> Technically this is a programming error, and a bug. But memory hasn't
>> actually been corrupted. The system properly stopped me from
>> corrupting memory. But my reward is that even though this fiber threw
>> an Error, and I get an error message in the log showing me the bug,
>> the web server itself is now out of commission. No other pages can be
>> served. This is like the equivalent of having a guard rail on a road
>> not only stop you from going off the cliff but proactively disable
>> your car afterwards to prevent you from more harm.
> [...]
>
> Isn't it customary to have the webserver launched by a script that
> restarts it whenever it crashes (after logging a message in an emergency
> logfile)?  Not an ideal solution, I know, but at least it minimizes
> downtime.

Yes, I can likely do this. This kills any existing connections being handled though, and is far far from ideal. It's also a hard crash, any operations such as writing DB data are killed mid-stream.

But you won't win over any minds that are used to php or python with this workaround.

>
> On another note, why didn't the compiler reject the above code? I
> thought it checks static arrays bounds at compile time whenever
> possible. Did I remember wrong?

I'm not sure, it's a toy example. In the real bug, the index was a variable. The annoying thing about this is that there is no actual memory corruption. It was properly stopped.

-Steve

On Wednesday, 31 May 2017 at 13:04:52 UTC, Steven Schveighoffer wrote:
> What are your thoughts? Have you run into this? If so, how did you solve it?

I don't use vibe, but my cgi.d just catches RangeError, kills the individual connection, and lets the others carry on. Can you do the same thing?

On 5/31/17 9:37 AM, Adam D. Ruppe wrote:
> On Wednesday, 31 May 2017 at 13:04:52 UTC, Steven Schveighoffer wrote:
>> What are your thoughts? Have you run into this? If so, how did you
>> solve it?
>
> I don't use vibe, but my cgi.d just catches RangeError, kills the
> individual connection, and lets the others carry on. Can you do the same
> thing?

There are a couple issues with this. At least from the perspective of vibe.d attempting to be a mainstream base library.

1. You can mark a function nothrow that throws a RangeError. So the compiler is free to assume the function won't throw and build faster code that won't properly clean up if an Error is thrown.

2. Technically, there is no guarantee by the runtime to unwind the stack. So at some point, your workaround may not even work. And even if it does, things like RAII may not work.

-Steve

Steven Schveighoffer wrote:

> On 5/31/17 9:37 AM, Adam D. Ruppe wrote:
>> On Wednesday, 31 May 2017 at 13:04:52 UTC, Steven Schveighoffer wrote:
>>> What are your thoughts? Have you run into this? If so, how did you
>>> solve it?
>>
>> I don't use vibe, but my cgi.d just catches RangeError, kills the
>> individual connection, and lets the others carry on. Can you do the same
>> thing?
>
> There are a couple issues with this. At least from the perspective of vibe.d attempting to be a mainstream base library.
>
> 1. You can mark a function nothrow that throws a RangeError. So the compiler is free to assume the function won't throw and build faster code that won't properly clean up if an Error is thrown.
>
> 2. Technically, there is no guarantee by the runtime to unwind the stack. So at some point, your workaround may not even work. And even if it does, things like RAII may not work.
>
> -Steve

that is, the question reduces to "should out-of-bounds be Error or Exception"?

i myself see no easy way to customize this with language attribute (new/delete disaster immediately comes to mind). so i'd say: "create your own array wrapper/implementation, and hope that all the functions you need are rangified, so they'll be able to work with YourArray".

On 5/31/17 9:54 AM, ketmar wrote:
> Steven Schveighoffer wrote:
>
>> On 5/31/17 9:37 AM, Adam D. Ruppe wrote:
>>> On Wednesday, 31 May 2017 at 13:04:52 UTC, Steven Schveighoffer wrote:
>>>> What are your thoughts? Have you run into this? If so, how did you
>>>> solve it?
>>>
>>> I don't use vibe, but my cgi.d just catches RangeError, kills the
>>> individual connection, and lets the others carry on. Can you do the same
>>> thing?
>>
>> There are a couple issues with this. At least from the perspective of
>> vibe.d attempting to be a mainstream base library.
>>
>> 1. You can mark a function nothrow that throws a RangeError. So the
>> compiler is free to assume the function won't throw and build faster
>> code that won't properly clean up if an Error is thrown.
>>
>> 2. Technically, there is no guarantee by the runtime to unwind the
>> stack. So at some point, your workaround may not even work. And even
>> if it does, things like RAII may not work.
>>
>
> that is, the question reduces to "should out-of-bounds be Error or
> Exception"?

That ship, unfortunately, has sailed. There is no reasonable migration path, as every function that uses indexing can currently be marked nothrow, and would stop compiling in one way or another. In other words mass breakage of every project would likely happen.

> i myself see no easy way to customize this with language attribute
> (new/delete disaster immediately comes to mind). so i'd say: "create
> your own array wrapper/implementation, and hope that all the functions
> you need are rangified, so they'll be able to work with YourArray".

I have, and it seems to work OK for my purposes (and wasn't really that bad actually).

Here is complete implementation (should be @safe too):

struct ExArr(T, size_t dim)
{
    T[dim] _value;
    alias _value this;
    ref inout(T) opIndex(size_t idx, string fname = __FILE__, size_t linenum = __LINE__) inout
    {
        if(idx >= dim)
            throw new Exception("Index out of bounds", fname, linenum);
        static ref x(ref inout(T[dim]) val, size_t i) @trusted { return val.ptr[i]; }
        return x(_value, idx);
    }
}

Now, I just need to search and replace for all the cases where I have a static array...

A dynamic array replacement shouldn't be too difficult either. Just need to override opIndex and opSlice. Then I can override those in my static array implementation as well.

-Steve

On 5/31/17 10:07 AM, Steven Schveighoffer wrote:

> Here is complete implementation (should be @safe too):
>
> struct ExArr(T, size_t dim)
> {
>     T[dim] _value;
>     alias _value this;
>     ref inout(T) opIndex(size_t idx, string fname = __FILE__, size_t
> linenum = __LINE__) inout
>     {
>         if(idx >= dim)
>             throw new Exception("Index out of bounds", fname, linenum);
>         static ref x(ref inout(T[dim]) val, size_t i) @trusted { return
> val.ptr[i]; }
>         return x(_value, idx);
>     }
> }

Just realized, that @trusted escape is just so unnecessarily verbose.

struct ExArr(T, size_t dim)
{
    T[dim] _value;
    alias _value this;
    ref inout(T) opIndex(size_t idx, string fname = __FILE__, size_t linenum = __LINE__) inout @trusted
    {
        if(idx >= dim)
            throw new Exception("Index out of bounds", fname, linenum);
        return _value.ptr[idx];
    }
}

-Steve

Steven Schveighoffer wrote:

> On 5/31/17 10:07 AM, Steven Schveighoffer wrote:
>
>> Here is complete implementation (should be @safe too):
>>
>> struct ExArr(T, size_t dim)
>> {
>>     T[dim] _value;
>>     alias _value this;
>>     ref inout(T) opIndex(size_t idx, string fname = __FILE__, size_t
>> linenum = __LINE__) inout
>>     {
>>         if(idx >= dim)
>>             throw new Exception("Index out of bounds", fname, linenum);
>>         static ref x(ref inout(T[dim]) val, size_t i) @trusted { return
>> val.ptr[i]; }
>>         return x(_value, idx);
>>     }
>> }
>
> Just realized, that @trusted escape is just so unnecessarily verbose.
>
> struct ExArr(T, size_t dim)
> {
>      T[dim] _value;
>      alias _value this;
>      ref inout(T) opIndex(size_t idx, string fname = __FILE__, size_t linenum = __LINE__) inout @trusted
>      {
>          if(idx >= dim)
>              throw new Exception("Index out of bounds", fname, linenum);
>          return _value.ptr[idx];
>      }
> }
>
> -Steve

bonus point: you can include index and length in error message! (something i really miss in dmd range error)

On 05/31/2017 09:34 AM, Steven Schveighoffer wrote:
> On 5/31/17 9:21 AM, H. S. Teoh via Digitalmars-d wrote:
>>
>> Isn't it customary to have the webserver launched by a script that
>> restarts it whenever it crashes (after logging a message in an emergency
>> logfile)?  Not an ideal solution, I know, but at least it minimizes
>> downtime.
> 
> Yes, I can likely do this. This kills any existing connections being handled though, and is far far from ideal. It's also a hard crash, any operations such as writing DB data are killed mid-stream.

Plus, relying on that strikes me as a DoS attack vector.