RejectedSoftware Forums

Sign up

RFC: Couple pull request ideas

I had a couple ideas for features I'm thinking of submitting pull reqs
for, but I wanted to bring them up and maybe discuss first before
actually going and coding them up:

  1. This one's simple: A callback that gets run on every HTTP request,
    just before the normal handler specified by the UrlRouter is called.
    Useful for any common processing, such as creating a session if there
    isn't already one, timing-out an old session while "keepalive-ing"
    active sessions, logging all requests or just all requests from a
    particular user or IP, etc.
  2. Consider this: A typical http request needs to be handled with a
    fairly strict:
    1. Send headers.
    2. Send exactly one content body.

There's a lot that can accidentally go wrong there. Headers/status can
be set after some piece of content is (accidentally) sent. You can have
a codepath that accidentally forgets to send any content at all (on a
non-HEAD request). You can have a codepath that accidentally sends two
HTML pages. Etc.

Ultimately, each handler (for the most part) needs to guarantee that
for all its code paths, it produces exactly one set of headers and one
content body. To me, that suggests "return value".

So I suggest this: An optional helper-handler that wraps a new type of
user-defined handler in which the user returns an HTTP result instead
of actually sending content and setting status codes directly. It would
work roughly like this:

// In user-code: //////////////////////////////////
HttpResult myHandler(HttpServerRequest req, SafeHttpServerResponse sres)
{

//res.statusCode = ...; // Error
//res.writeBody(...);   // Error

// Can't screw up by forgetting to send a response,
// or sending two responses, or sending a response
// and then changing a header/status code, etc.
//
// Also, this is much cleaner/nicer than:
// if(blah)
// {
//     httpNotFound(res);
//     return;
// }
if(blah)
    return httpNotFound();
else if(blah2)
    return httpRedirect("/url/path");

return httpOK("Hello!", "text/plain");

}

auto router = new UrlRouter();
router.get("/url/path", &safeHandler!myHandler);

// In Vibe.d: //////////////////////////////////
struct HttpResult
{

int statusCode;
string mime;
string[string] headers;

//TODO: Also support ubyte[] and InputRange.
string content;

}

HttpResult httpNotFound()
{

HttpResult r;
r.statusCode = HttpStatus.NotFound;
r.mime = "text/html";
r.content = /+...some default or user-customized page...+/;
return r;

}

HttpResult httpRedirect(string url, int status = HttpStatus.Found)
{

HttpResult r;
r.statusCode = status;
r.locationHeader = url;
r.mime = "text/plain";
r.content = "Redirect to: " ~ url;
return r;

}

HttpResult httpOK(string content, string mimeType)
{

HttpResult r;
r.statusCode = HttpStatus.OK;
r.mime = mimeType;
r.content = content;
return r;

}

final class SafeHttpServerResponse
{

// Wraps a HttpServerResponse, but
// blocks 'statusCode', 'writeBody', etc.
//
// Or maybe Response is entirely omittable?

}

void safeHandler(alias userHanderDg)(HttpServerRequest req,
HttpServerResponse res)
{

HttpResult result = userHanderDg(req, new

SafeHttpServerResponse(res));

// Send header/status/content on behalf of user's handler.
res.statusCode = result.statusCode;
res.headers = result.headers;
res.writeBody(result.content, result.mime);

}

I'm already doing more-or-less that in my own Vibe.d apps, but I think
it would be a good thing to have in Vibe.d proper. Your thoughts?

Re: RFC: Couple pull request ideas

I really like idea 2 (having handlers return something instead of using responses directly), it is very .NETish, but this may not be a bad thing. Haven't delved into your implementation though.

As for 1, I thought this was possible using injectors?

Re: RFC: Couple pull request ideas

Am 18.03.2013 18:50, schrieb Nick Sabalausky:

I had a couple ideas for features I'm thinking of submitting pull reqs
for, but I wanted to bring them up and maybe discuss first before
actually going and coding them up:

  1. This one's simple: A callback that gets run on every HTTP request,

just before the normal handler specified by the UrlRouter is called.
Useful for any common processing, such as creating a session if there
isn't already one, timing-out an old session while "keepalive-ing"
active sessions, logging all requests or just all requests from a
particular user or IP, etc.

If you are using UrlRouter, you can already do that like this:

auto router = new UrlRouter;
router.any("*", &performOnAllRequests);
router.get("/", &showIndexPage);
// ...

If performOnAllRequests doesn't write a response, the router will
continue to match the request against the following routes until some of
them actually writes something or throws an exception.

Another possibility is calling listenHttp with a delegate that does
the special request handling and then call router.handleRequest(req,<br>res) to continue with the usual route matching.

  1. Consider this: A typical http request needs to be handled with a

fairly strict:

1. Send headers.
2. Send exactly one content body.

There's a lot that can accidentally go wrong there. Headers/status can
be set after some piece of content is (accidentally) sent. You can have
a codepath that accidentally forgets to send any content at all (on a
non-HEAD request). You can have a codepath that accidentally sends two
HTML pages. Etc.

Ultimately, each handler (for the most part) needs to guarantee that
for all its code paths, it produces exactly one set of headers and one
content body. To me, that suggests "return value".

So I suggest this: An optional helper-handler that wraps a new type of
user-defined handler in which the user returns an HTTP result instead
of actually sending content and setting status codes directly. It would
work roughly like this:

(... code ...)

I'm already doing more-or-less that in my own Vibe.d apps, but I think
it would be a good thing to have in Vibe.d proper. Your thoughts?

But in that form it would mean that the response body needs to stay in
memory and even if r.content is replaced by some kind of callback or
stream it would mean that there is at least one GC allocation. Avoiding
memory allocations is the main reason why the current interface is like
it is.

While I agree that this is a good interface for certain applications,
what I'm struggling a bit with is including multiple ways to achieve the
same thing in the library... while I have to admit that there are
already some high level things, the plan was rather to keep vibe.d
itself basic and to the point (the reason why it contains those things
is mainly because dub didn't exist yet and it would have made the build
process for certain projects more complex, but they will become separate
packages at some point).

Btw. instead of

if(blah){
	httpNotFound(res);
	return;
}

this also works right now:

if(blah) throw new HttpStatusException(HttpStatus.notFound);

or shorter

enforceHttp(!blah, HttpStatus.notFound);

Re: RFC: Couple pull request ideas

On Tue, 19 Mar 2013 07:44:01 +0100
Sönke Ludwig sludwig@rejectedsoftware.com wrote:

If you are using UrlRouter, you can already do that like this:

auto router = new UrlRouter;
router.any("*", &performOnAllRequests);
router.get("/", &showIndexPage);
// ...

If performOnAllRequests doesn't write a response, the router will
continue to match the request against the following routes until some
of them actually writes something or throws an exception.

Another possibility is calling listenHttp with a delegate that does
the special request handling and then call router.handleRequest(req,<br>res) to continue with the usual route matching.

Ah, ok, I didn't realize any of that. Sounds good.

But in that form it would mean that the response body needs to stay in
memory and even if r.content is replaced by some kind of callback or
stream it would mean that there is at least one GC allocation.
Avoiding memory allocations is the main reason why the current
interface is like it is.

While I agree that this is a good interface for certain applications,
what I'm struggling a bit with is including multiple ways to achieve
the same thing in the library... while I have to admit that there are
already some high level things, the plan was rather to keep vibe.d
itself basic and to the point (the reason why it contains those things
is mainly because dub didn't exist yet and it would have made the
build process for certain projects more complex, but they will become
separate packages at some point).

Btw. instead of

if(blah){
	httpNotFound(res);
	return;
}

this also works right now:

if(blah) throw new HttpStatusException(HttpStatus.notFound);

or shorter

enforceHttp(!blah, HttpStatus.notFound);



Hmm. I see.