Skip to content

Commit

Permalink
Merge pull request #1255 from basho/feature/list-objs-any-bytes
Browse files Browse the repository at this point in the history
Allow any bytes (including non-UTF8 ones) in List Objects response XML

Reviewed-by: kuenishi
  • Loading branch information
borshop committed Oct 6, 2015
2 parents 4c215f6 + b28fec7 commit 34435ef
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 61 deletions.
2 changes: 1 addition & 1 deletion src/riak_cs_wm_bucket_location.erl
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ to_xml(RD, Ctx=#context{user=User,bucket=Bucket}) ->
Doc = [{'LocationConstraint',
[{xmlns, "http://s3.amazonaws.com/doc/2006-03-01/"}],
[riak_cs_config:region()]}],
{riak_cs_xml:export_xml(Doc), RD, Ctx}
{riak_cs_xml:to_xml(Doc), RD, Ctx}
end.


108 changes: 48 additions & 60 deletions src/riak_cs_xml.erl
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@

%% Public API
-export([scan/1,
export_xml/1,
to_xml/1]).

-define(XML_SCHEMA_INSTANCE, "http://www.w3.org/2001/XMLSchema-instance").
Expand All @@ -55,6 +54,17 @@
xmlPI() | xmlDocument().
-export_type([xmlNode/0, xmlElement/0, xmlText/0]).

%% Types for simple forms
-type tag() :: atom().
-type content() :: [element()].
-type element() :: {tag(), attributes(), content()} |
{tag(), content()} |
tag() |
iodata() |
integer() |
float(). % Really Needed?
-type simple_form() :: content().

%% ===================================================================
%% Public API
%% ===================================================================
Expand All @@ -72,13 +82,6 @@ scan(String) ->
_E -> {error, malformed_xml}
end.

%% This function is temporary and should be removed once all XML
%% handling has been moved into this module.
%% @TODO Remove this asap!
export_xml(XmlDoc) ->
unicode:characters_to_binary(
xmerl:export_simple(XmlDoc, xmerl_xml, [{prolog, ?XML_PROLOG}]), unicode, unicode).

-spec to_xml(term()) -> binary().
to_xml(undefined) ->
[];
Expand All @@ -91,7 +94,8 @@ to_xml(#acl_v1{}=Acl) ->
to_xml(?LBRESP{}=ListBucketsResp) ->
list_buckets_response_to_xml(ListBucketsResp);
to_xml(?LORESP{}=ListObjsResp) ->
list_objects_response_to_xml(ListObjsResp);
SimpleForm = list_objects_response_to_simple_form(ListObjsResp),
to_xml(SimpleForm);
to_xml(?RCS_USER{}=User) ->
user_record_to_xml(User);
to_xml({users, Users}) ->
Expand All @@ -101,7 +105,11 @@ to_xml({users, Users}) ->
%% Internal functions
%% ===================================================================

export_xml(XmlDoc) ->
list_to_binary(xmerl:export_simple(XmlDoc, xmerl_xml, [{prolog, ?XML_PROLOG}])).

%% @doc Convert simple form into XML.
-spec simple_form_to_xml(simple_form()) -> iodata().
simple_form_to_xml(Elements) ->
XmlDoc = format_elements(Elements),
export_xml(XmlDoc).
Expand All @@ -116,8 +124,7 @@ format_element({Tag, Attrs, Elements}) ->
format_element(Value) ->
format_value(Value).

%% @doc Convert an internal representation of an ACL
%% into XML.
%% @doc Convert an internal representation of an ACL into XML.
-spec acl_to_xml(acl()) -> binary().
acl_to_xml(Acl) ->
Content = [make_internal_node('Owner', owner_content(acl_owner(Acl))),
Expand All @@ -143,25 +150,34 @@ owner_content({OwnerName, OwnerId}) ->
[make_external_node('ID', OwnerId),
make_external_node('DisplayName', OwnerName)].

list_objects_response_to_xml(Resp) ->
KeyContents = [key_content_to_xml(Content) ||
Content <- (Resp?LORESP.contents)],
CommonPrefixes = [common_prefix_to_xml(Prefix) ||
Prefix <- Resp?LORESP.common_prefixes],
Contents = [make_external_node('Name', Resp?LORESP.name),
make_external_node('Prefix', Resp?LORESP.prefix),
make_external_node('Marker', Resp?LORESP.marker)] ++
list_objects_response_to_simple_form(Resp) ->
KeyContents = [{'Contents', key_content_to_simple_form(Content)} ||
Content <- (Resp?LORESP.contents)],
CommonPrefixes = [{'CommonPrefixes', [{'Prefix', [CommonPrefix]}]} ||
CommonPrefix <- Resp?LORESP.common_prefixes],
Contents = [{'Name', [Resp?LORESP.name]},
{'Prefix', [Resp?LORESP.prefix]},
{'Marker', [Resp?LORESP.marker]}] ++
%% use a list-comprehension trick to only include
%% the `NextMarker' element if it's not `undefined'
[make_external_node('NextMarker', NextMarker) ||
NextMarker <- [Resp?LORESP.next_marker],
NextMarker =/= undefined] ++
[make_external_node('MaxKeys', Resp?LORESP.max_keys),
make_external_node('Delimiter', Resp?LORESP.delimiter),
make_external_node('IsTruncated', Resp?LORESP.is_truncated)] ++
[{'NextMarker', [NextMarker]} ||
NextMarker <- [Resp?LORESP.next_marker],
NextMarker =/= undefined] ++
[{'MaxKeys', [Resp?LORESP.max_keys]},
{'Delimiter', [Resp?LORESP.delimiter]},
{'IsTruncated', [Resp?LORESP.is_truncated]}] ++
KeyContents ++ CommonPrefixes,
export_xml([make_internal_node('ListBucketResult', [{'xmlns', ?S3_XMLNS}],
Contents)]).
[{'ListBucketResult', [{'xmlns', ?S3_XMLNS}], Contents}].

key_content_to_simple_form(KeyContent) ->
#list_objects_owner_v1{id=Id, display_name=Name} = KeyContent?LOKC.owner,
[{'Key', [KeyContent?LOKC.key]},
{'LastModified', [KeyContent?LOKC.last_modified]},
{'ETag', [KeyContent?LOKC.etag]},
{'Size', [KeyContent?LOKC.size]},
{'StorageClass', [KeyContent?LOKC.storage_class]},
{'Owner', [{'ID', [Id]},
{'DisplayName', [Name]}]}].

list_buckets_response_to_xml(Resp) ->
BucketsContent =
Expand All @@ -183,23 +199,8 @@ bucket_to_xml(Name, CreationDate) ->
make_external_node('CreationDate', CreationDate)]).

user_to_xml_owner(?RCS_USER{canonical_id=CanonicalId, display_name=Name}) ->
make_internal_node('Owner', [make_external_node('ID', CanonicalId),
make_external_node('DisplayName', Name)]).

key_content_to_xml(KeyContent) ->
Contents =
[make_external_node('Key', KeyContent?LOKC.key),
make_external_node('LastModified', KeyContent?LOKC.last_modified),
make_external_node('ETag', KeyContent?LOKC.etag),
make_external_node('Size', KeyContent?LOKC.size),
make_external_node('StorageClass', KeyContent?LOKC.storage_class),
make_owner(KeyContent?LOKC.owner)],
make_internal_node('Contents', Contents).

-spec common_prefix_to_xml(binary()) -> internal_node().
common_prefix_to_xml(CommonPrefix) ->
make_internal_node('CommonPrefixes',
[make_external_node('Prefix', CommonPrefix)]).
make_internal_node('Owner', [make_external_node('ID', [CanonicalId]),
make_external_node('DisplayName', [Name])]).

-spec make_internal_node(atom(), term()) -> internal_node().
make_internal_node(Name, Content) ->
Expand Down Expand Up @@ -252,31 +253,18 @@ make_grant(DisplayName, CanonicalId, Permission) ->
make_external_node('Permission', Permission)],
make_internal_node('Grant', GrantContent).

-spec make_owner(list_objects_owner()) -> internal_node().
make_owner(#list_objects_owner_v1{id=Id, display_name=Name}) ->
Content = [make_external_node('ID', Id),
make_external_node('DisplayName', Name)],
make_internal_node('Owner', Content).

-spec format_value(atom() | integer() | binary() | list()) -> string().
%% @doc Format value depending on its type
%% Handling of characters (binaries or lists of integers) are as follows:
%% - For binaries, we assume that they are encoded by UTF-8.
%% - For lists, we assume they are converted from UTF-8 encoded binaries
%% by applying binary_to_list/1.
%% This is tricky point but binary_to_list/1 treats binaries as if it is
%% Latin-1 encoded, so we should turn them back by list_to_binary first.
%% Then we can apply unicode:characters_to_binary safely.
%% @doc Convert value depending on its type into strings
format_value(undefined) ->
[];
format_value(Val) when is_atom(Val) ->
atom_to_list(Val);
format_value(Val) when is_binary(Val) ->
unicode:characters_to_list(Val, unicode);
binary_to_list(Val);
format_value(Val) when is_integer(Val) ->
integer_to_list(Val);
format_value(Val) when is_list(Val) ->
format_value(list_to_binary(Val));
Val;
format_value(Val) when is_float(Val) ->
io_lib:format("~p", [Val]).

Expand Down

0 comments on commit 34435ef

Please sign in to comment.