Create a bucket props validation module and register it

src/riak_kv_app.erl

@@ -79,6 +79,9 @@ start(_Type, _StartArgs) ->
     %% the app is missing or packaging is broken.
     catch cluster_info:register_app(riak_kv_cinfo),
 
+    %% Register the bucket validation fun
+     riak_core:register([{bucket_validator, riak_kv_bucket}]),
+
     %% Spin up supervisor
     case riak_kv_sup:start_link() of
         {ok, Pid} ->

src/riak_kv_bucket.erl

@@ -0,0 +1,104 @@
+%% -------------------------------------------------------------------
+%%
+%% riak_kv_bucket: bucket validation functions
+%%
+%% Copyright (c) 2007-2011 Basho Technologies, Inc.  All Rights Reserved.
+%%
+%% This file is provided to you under the Apache License,
+%% Version 2.0 (the "License"); you may not use this file
+%% except in compliance with the License.  You may obtain
+%% a copy of the License at
+%%
+%%   http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing,
+%% software distributed under the License is distributed on an
+%% "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+%% KIND, either express or implied.  See the License for the
+%% specific language governing permissions and limitations
+%% under the License.
+%%
+%% -------------------------------------------------------------------
+%% @doc KV Bucket validation functions
+
+-module(riak_kv_bucket).
+
+-export([validate/1]).
+
+-include_lib("eunit/include/eunit.hrl").
+
+-type prop() :: {PropName::atom(), PropValue::any()}.
+-type error() :: {PropName::atom(), ErrorReason::atom()}.
+-type props() :: [prop()].
+-type errors() :: [error()].
+
+-spec validate(props()) -> {props(), errors()}.
+validate(BucketProps) when is_list(BucketProps) ->
+    lager:debug("Was called ~p~n", [BucketProps]),
+    validate(BucketProps, [], []).
+
+-spec validate(InProps::props(), ValidProps::props(), Errors::errors()) ->
+                      {props(), errors()}.
+validate([], ValidProps, Errors) ->
+    {ValidProps, Errors};
+validate([{BoolProp, MaybeBool}|T], ValidProps, Errors) when is_atom(BoolProp), BoolProp =:= allow_mult
+                                                             orelse BoolProp =:= basic_quorum
+                                                             orelse BoolProp =:= last_write_wins
+                                                             orelse BoolProp =:= notfound_ok  ->
+    case coerce_bool(MaybeBool) of 
+        error ->
+            validate(T, ValidProps, [{BoolProp, not_boolean}|Errors]);
+        Bool ->
+            validate(T, [{BoolProp, Bool}|ValidProps], Errors)
+    end;
+validate([Prop|T], ValidProps, Errors) ->
+    validate(T, [Prop|ValidProps], Errors).
+
+-spec coerce_bool(any()) -> boolean() | error.
+coerce_bool(true) ->
+    true;
+coerce_bool(false) ->
+    false;
+coerce_bool(MaybeBool) when is_atom(MaybeBool) ->
+    coerce_bool(atom_to_list(MaybeBool));
+coerce_bool(MaybeBool) when is_binary(MaybeBool) ->
+    coerce_bool(binary_to_list(MaybeBool));
+coerce_bool(Int) when is_integer(Int), Int =< 0 ->
+    false;
+coerce_bool(Int) when is_integer(Int) , Int > 0 ->
+    true;
+coerce_bool(MaybeBool) when is_list(MaybeBool) ->
+    Lower = string:to_lower(MaybeBool),
+    Atom = (catch list_to_existing_atom(Lower)),
+    case Atom of 
+        true -> true;
+        false -> false;
+        _ -> error
+    end;
+coerce_bool(_) ->
+    error.
+
+%%
+%% EUNIT tests...
+%%
+
+-ifdef (TEST).
+
+coerce_bool_test_ () ->
+    [?_assertEqual(false, coerce_bool(false)),
+     ?_assertEqual(true, coerce_bool(true)),
+     ?_assertEqual(true, coerce_bool("True")),
+     ?_assertEqual(false, coerce_bool("fAlSE")),
+     ?_assertEqual(false, coerce_bool(<<"FAlse">>)),
+     ?_assertEqual(true, coerce_bool(<<"trUe">>)),
+     ?_assertEqual(true, coerce_bool(1)),     
+     ?_assertEqual(true, coerce_bool(234567)),
+     ?_assertEqual(false, coerce_bool(0)),
+     ?_assertEqual(false, coerce_bool(-1234)),
+     ?_assertEqual(false, coerce_bool('FALSE')),
+     ?_assertEqual(true, coerce_bool('TrUe')),
+     ?_assertEqual(error, coerce_bool("Purple")),
+     ?_assertEqual(error, coerce_bool(<<"frangipan">>)),
+     ?_assertEqual(error, coerce_bool(erlang:make_ref()))
+    ].
+-endif.

src/riak_kv_pb_socket.erl

@@ -441,8 +441,12 @@ process_message(#rpbgetbucketreq{bucket=B},
 process_message(#rpbsetbucketreq{bucket=B, props = PbProps}, 
                 #state{client=C} = State) ->
     Props = riakc_pb:erlify_rpbbucketprops(PbProps),
-    ok = C:set_bucket(B, Props),
-    send_msg(rpbsetbucketresp, State);
+    case C:set_bucket(B, Props) of
+        ok ->
+            send_msg(rpbsetbucketresp, State);
+        {error, Details} ->
+            send_error("Invalid bucket properties: ~p", [Details], State)
+    end;
 
 %% TODO: refactor, cleanup
 %% Start map/reduce job - results will be processed in handle_info

src/riak_kv_wm_props.erl

@@ -195,8 +195,14 @@ get_bucket_props_json(Client, Bucket) ->
 %%      bucket-level PUT request.
 accept_bucket_body(RD, Ctx=#ctx{bucket=B, client=C, bucketprops=Props}) ->
     ErlProps = lists:map(fun erlify_bucket_prop/1, Props),
-    C:set_bucket(B, ErlProps),
-    {true, RD, Ctx}.
+    case C:set_bucket(B, ErlProps) of
+        ok ->
+            {true, RD, Ctx};
+        {error, Details} ->
+            JSON = mochijson2:encode(Details),
+            RD2 = wrq:append_to_resp_body(JSON, RD),
+            {{halt, 400}, RD2, Ctx}
+    end.
 
 %% @spec jsonify_bucket_prop({Property::atom(), erlpropvalue()}) ->
 %%           {Property::binary(), jsonpropvalue()}
@@ -297,7 +303,5 @@ erlify_bucket_prop({?JSON_CHASH, {struct, Props}}) ->
                     list_to_existing_atom(
                       binary_to_list(
                         proplists:get_value(?JSON_FUN, Props)))}};
-erlify_bucket_prop({?JSON_ALLOW_MULT, Value}) ->
-    {allow_mult, riak_kv_wm_utils:any_to_bool(Value)};
 erlify_bucket_prop({Prop, Value}) ->
     {list_to_existing_atom(binary_to_list(Prop)), Value}.