On Sun, 04 May 2014 20:04:33 GMT, Sönke Ludwig wrote:

With today's API, the following should work for all the corner cases (at least if there are no bugs in fullURL): 

void redirect(HTTPServerRequest req, HTTPServerResponse res)
{
    auto url = req.fullURL;
    url.host = "www.example.com";
    res.redirect(url);
}

A redirectSite function could be useful, but it would also have to support the case, where there is a different port/protocol/path prefix compared to the original URL, so I'm not sure how much value it would actually add.

The property HTTPServerRequest.fullURL does not work correctly with HTTP 1.0 (no Host header) or if you pass it an empty host header (e.g., wget --header="Host:"), causing the server to crash in the line url.host = url.host.split(":")[0];, due to url.host being null, the split result having 0 length, and therefore the [0] indexing generating a range violation error. 

https://github.com/rejectedsoftware/vibe.d/issues/786

So, how should fullURL be fixed? I guess this would work: 

if(url.host !is null)
    url.host = url.host.split(":")[0];
else
    url.host = "";

Should the else be there, to avoid a null string? Or would a null string be fine, meaning no host information present in the request?

Regarding the redirect functionality, first of all we are doing busy work trying to fill the host field (from the X-Forwarded-Host or the Host header) when we are actually going to override the host. This concerns me even more because we are allocating from the GC in the call to std.array.split(), producing garbage that isn't used even once. I guess we could use std.algorithm.findSplit / splitter to avoid the GC allocation, but is that the best we can do?

In any case, I think the problems I had with this seemingly trivial implementation of a redirect functionality only further confirm that such a redirect function should be provided by vibe.d itself. The implementation is not obvious (if you want to preserve the protocol, port and path, etc.) even if it is simple. And it might not be that simple, maybe we'll find further problems along the line. If the redirect is part of vibe.d, then we can fix all the clients by fixing vibe.d. Or if we provide a better version in the future that avoids the busy work of trying to fill the host field, then all the clients get the better performing version. Etc.

BTW, does it really make sense to allow the whole server to crash if we try to index an array out of bounds in the HTTP request handler? (in @safe and bound-checked handlers, at least). Most servers using memory safe languages seem to only produce an HTTP 500 for the specific request (e.g., Django, Node, etc.). Why should vibe.d be any different? I know that *Error exceptions are generally meant not to be recoverable, but I guess that in this context that policy does not make a lot of sense. Is there any situation where just producing an HTTP 500 would be unsafe?

I had some further questions that arose when I debugged this issue, but I think I should start a new thread for those. Thanks.