Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Return "Bad Request" for errors caused by client #827

Merged
merged 1 commit into from
Sep 20, 2014
Merged

Return "Bad Request" for errors caused by client #827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions source/vibe/http/auth/basic_auth.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ HTTPServerRequestDelegate performBasicAuth(string realm, bool delegate(string us
string user_pw = cast(string)Base64.decode((*pauth)[6 .. $]);

auto idx = user_pw.indexOf(":");
enforce(idx >= 0, "Invalid auth string format!");
enforceBadRequest(idx >= 0, "Invalid auth string format!");
string user = user_pw[0 .. idx];
string password = user_pw[idx+1 .. $];

Expand Down Expand Up @@ -69,7 +69,7 @@ string performBasicAuth(HTTPServerRequest req, HTTPServerResponse res, string re
string user_pw = cast(string)Base64.decode((*pauth)[6 .. $]);

auto idx = user_pw.indexOf(":");
enforce(idx >= 0, "Invalid auth string format!");
enforceBadRequest(idx >= 0, "Invalid auth string format!");
string user = user_pw[0 .. idx];
string password = user_pw[idx+1 .. $];

Expand Down
22 changes: 15 additions & 7 deletions source/vibe/http/common.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ void enforceHTTP(T)(T condition, HTTPStatus statusCode, string message = null)
enforce(condition, new HTTPStatusException(statusCode, message));
}

/**
Utility function that throws a HTTPStatusException with status code "400 Bad Request" if the _condition is not met.
*/
void enforceBadRequest(T)(T condition, string message = null)
{
enforceHTTP(condition, HTTPStatus.badRequest, message);
}


/**
Represents an HTTP request made to a server.
Expand Down Expand Up @@ -285,14 +293,14 @@ string getHTTPVersionString(HTTPVersion ver)

HTTPVersion parseHTTPVersion(ref string str)
{
enforce(str.startsWith("HTTP/"));
enforceBadRequest(str.startsWith("HTTP/"));
str = str[5 .. $];
int majorVersion = parse!int(str);
enforce(str.startsWith("."));
enforceBadRequest(str.startsWith("."));
str = str[1 .. $];
int minorVersion = parse!int(str);

enforce( majorVersion == 1 && (minorVersion == 0 || minorVersion == 1) );
enforceBadRequest( majorVersion == 1 && (minorVersion == 0 || minorVersion == 1) );
return minorVersion == 0 ? HTTPVersion.HTTP_1_0 : HTTPVersion.HTTP_1_1;
}

Expand Down Expand Up @@ -327,9 +335,9 @@ final class ChunkedInputStream : InputStream {

void read(ubyte[] dst)
{
enforce(!empty, "Read past end of chunked stream.");
enforceBadRequest(!empty, "Read past end of chunked stream.");
while( dst.length > 0 ){
enforce(m_bytesInCurrentChunk > 0, "Reading past end of chunked HTTP stream.");
enforceBadRequest(m_bytesInCurrentChunk > 0, "Reading past end of chunked HTTP stream.");

auto sz = cast(size_t)min(m_bytesInCurrentChunk, dst.length);
m_in.read(dst[0 .. sz]);
Expand All @@ -340,7 +348,7 @@ final class ChunkedInputStream : InputStream {
// skip current chunk footer and read next chunk
ubyte[2] crlf;
m_in.read(crlf);
enforce(crlf[0] == '\r' && crlf[1] == '\n');
enforceBadRequest(crlf[0] == '\r' && crlf[1] == '\n');
readChunk();
}
}
Expand All @@ -360,7 +368,7 @@ final class ChunkedInputStream : InputStream {
// skip final chunk footer
ubyte[2] crlf;
m_in.read(crlf);
enforce(crlf[0] == '\r' && crlf[1] == '\n');
enforceBadRequest(crlf[0] == '\r' && crlf[1] == '\n');
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions source/vibe/http/form.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ private HTTPServerRequestDelegate formMethodHandler(DelegateType)(DelegateType f
void handler(HTTPServerRequest req, HTTPServerResponse res)
{
string error;
enforce(applyParametersFromAssociativeArray(req, res, func, error, strict), error);
enforceBadRequest(applyParametersFromAssociativeArray(req, res, func, error, strict), error);
}
return &handler;
}
Expand Down Expand Up @@ -232,7 +232,7 @@ private HTTPServerRequestDelegate formMethodHandler(T, string method)(T inst, Fl
}
errors~="Overload "~method~typeid(ParameterTypeTuple!func).toString()~" failed: "~error~"\n\n";
}
enforce(false, "No method found that matches the found form data:\n"~errors);
enforceBadRequest(false, "No method found that matches the found form data:\n"~errors);
}
return &handler;
}
Expand Down Expand Up @@ -449,7 +449,7 @@ struct StringLengthCountingRange {
// We have a default value for country if not provided, so we don't care that it is not:
p.address.country="Important Country";
p.name="Jane";
enforce(loadFormData(req, p, "customer"), "More data than needed provided!");
enforceBadRequest(loadFormData(req, p, "customer"), "More data than needed provided!");
// p will now contain the provided form data, non provided data stays untouched.
assert(p.address.country=="Important Country");
assert(p.name=="John");
Expand Down
12 changes: 6 additions & 6 deletions source/vibe/http/server.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1351,11 +1351,11 @@ private bool handleRequest(Stream http_stream, TCPConnection tcp_connection, HTT
if (auto pcl = "Content-Length" in req.headers) {
string v = *pcl;
auto contentLength = parse!ulong(v); // DMDBUG: to! thinks there is a H in the string
enforce(v.length == 0, "Invalid content-length");
enforce(settings.maxRequestSize <= 0 || contentLength <= settings.maxRequestSize, "Request size too big");
enforceBadRequest(v.length == 0, "Invalid content-length");
enforceBadRequest(settings.maxRequestSize <= 0 || contentLength <= settings.maxRequestSize, "Request size too big");
limited_http_input_stream = FreeListRef!LimitedHTTPInputStream(reqReader, contentLength);
} else if (auto pt = "Transfer-Encoding" in req.headers) {
enforce(*pt == "chunked");
enforceBadRequest(*pt == "chunked");
chunked_input_stream = FreeListRef!ChunkedInputStream(reqReader);
limited_http_input_stream = FreeListRef!LimitedHTTPInputStream(chunked_input_stream, settings.maxRequestSize, true);
} else {
Expand Down Expand Up @@ -1506,13 +1506,13 @@ private void parseRequestHeader(HTTPServerRequest req, InputStream http_stream,

//Method
auto pos = reqln.indexOf(' ');
enforce(pos >= 0, "invalid request method");
enforceBadRequest(pos >= 0, "invalid request method");

req.method = httpMethodFromString(reqln[0 .. pos]);
reqln = reqln[pos+1 .. $];
//Path
pos = reqln.indexOf(' ');
enforce(pos >= 0, "invalid request path");
enforceBadRequest(pos >= 0, "invalid request path");

req.requestURL = reqln[0 .. pos];
reqln = reqln[pos+1 .. $];
Expand All @@ -1531,7 +1531,7 @@ private void parseCookies(string str, ref CookieValueMap cookies)
{
while(str.length > 0) {
auto idx = str.indexOf('=');
enforce(idx > 0, "Expected name=value.");
enforceBadRequest(idx > 0, "Expected name=value.");
string name = str[0 .. idx].strip();
str = str[idx+1 .. $];

Expand Down
2 changes: 1 addition & 1 deletion source/vibe/web/common.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ package T webConvTo(T)(string str)
static if (is(typeof(T.fromStringValidate(str, &error)))) {
static assert(is(typeof(T.fromStringValidate(str, &error)) == Nullable!T));
auto ret = T.fromStringValidate(str, &error);
enforce(!ret.isNull(), error); // TODO: refactor internally to work without exceptions
enforceBadRequest(!ret.isNull(), error); // TODO: refactor internally to work without exceptions
return ret.get();
} else static if (is(typeof(T.fromString(str)))) {
static assert(is(typeof(T.fromString(str)) == T));
Expand Down
41 changes: 23 additions & 18 deletions source/vibe/web/rest.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -500,10 +500,9 @@ private HTTPServerRequestDelegate jsonMethodHandler(T, string method, alias Func
ParameterDefaultValueTuple, ParameterIdentifierTuple;
import std.string : format;
import std.algorithm : startsWith;
import std.exception : enforce;

import vibe.http.server : HTTPServerRequest, HTTPServerResponse;
import vibe.http.common : HTTPStatusException, HTTPStatus;
import vibe.http.common : HTTPStatusException, HTTPStatus, enforceBadRequest;
import vibe.utils.string : sanitizeUTF8;
import vibe.internal.meta.funcattr : IsAttributedParameter;

Expand Down Expand Up @@ -541,7 +540,7 @@ private HTTPServerRequestDelegate jsonMethodHandler(T, string method, alias Func
} else static if (ParamNames[i].startsWith("_")) {
// URL parameter
static if (ParamNames[i] != "_dummy") {
enforce(
enforceBadRequest(
ParamNames[i][1 .. $] in req.params,
format("req.param[%s] was not set!", ParamNames[i][1 .. $])
);
Expand All @@ -557,8 +556,8 @@ private HTTPServerRequestDelegate jsonMethodHandler(T, string method, alias Func
logDebug("query %s of %s", pname, req.query);

static if (is (DefVal == void)) {
enforce(
pname in req.query,
enforceBadRequest(
ParamNames[i] in req.query,
format("Missing query parameter '%s'", pname)
);
} else {
Expand All @@ -572,22 +571,22 @@ private HTTPServerRequestDelegate jsonMethodHandler(T, string method, alias Func
} else {
logDebug("%s %s", method, pname);

enforce(
enforceBadRequest(
req.contentType == "application/json",
"The Content-Type header needs to be set to application/json."
);
enforce(
enforceBadRequest(
req.json.type != Json.Type.Undefined,
"The request body does not contain a valid JSON value."
);
enforce(
enforceBadRequest(
req.json.type == Json.Type.Object,
"The request body must contain a JSON object with an entry for each parameter."
);

static if (is(DefVal == void)) {
enforce(
req.json[pname].type != Json.Type.Undefined,
enforceBadRequest(
req.json[ParamNames[i]].type != Json.Type.Undefined,
format("Missing parameter %s", pname)
);
} else {
Expand Down Expand Up @@ -957,14 +956,20 @@ private {
T fromRestString(T)(string value)
{
import std.traits;
static if (isInstanceOf!(Nullable, T)) return T(fromRestString!(typeof(T.init.get()))(value));
else static if( is(T == bool) ) return value == "true";
else static if( is(T : int) ) return to!T(value);
else static if( is(T : double) ) return to!T(value); // FIXME: formattedWrite(dst, "%.16g", json.get!double);
else static if( is(T : string) ) return value;
else static if( __traits(compiles, T.fromISOExtString("hello")) ) return T.fromISOExtString(value);
else static if( __traits(compiles, T.fromString("hello")) ) return T.fromString(value);
else return deserializeJson!T(parseJson(value));
import std.conv : ConvException;
import vibe.web.common : HTTPStatusException, HTTPStatus;
try {
static if (isInstanceOf!(Nullable, T)) return T(fromRestString!(typeof(T.init.get()))(value));
else static if( is(T == bool) ) return value == "true";
else static if( is(T : int) ) return to!T(value);
else static if( is(T : double) ) return to!T(value); // FIXME: formattedWrite(dst, "%.16g", json.get!double);
else static if( is(T : string) ) return value;
else static if( __traits(compiles, T.fromISOExtString("hello")) ) return T.fromISOExtString(value);
else static if( __traits(compiles, T.fromString("hello")) ) return T.fromString(value);
else return deserializeJson!T(parseJson(value));
} catch(ConvException e) {
throw new HTTPStatusException(HTTPStatus.badRequest, e.msg);
}
}
}

Expand Down