Fix URL escaping and GET url reporting for mod_http_upload.

apps/ejabberd/src/http_upload/mod_http_upload.erl

@@ -175,7 +175,7 @@ compose_iq_reply(IQ, Namespace, PutUrl, GetUrl, Headers) ->
               name     = <<"slot">>,
               attrs    = [{<<"xmlns">>, Namespace}],
               children = [create_url_xmlel(<<"put">>, PutUrl, Headers, Namespace),
-                          create_url_xmlel(<<"get">>, PutUrl, #{}, Namespace)]},
+                          create_url_xmlel(<<"get">>, GetUrl, #{}, Namespace)]},
     IQ#iq{type = result, sub_el =[Slot]}.
 
 
@@ -210,7 +210,8 @@ generate_token(Host) ->
 -spec file_too_large_error(MaxFileSize :: non_neg_integer(), Namespace :: binary()) -> jlib:exml().
 file_too_large_error(MaxFileSize, Namespace) ->
     MaxFileSizeBin = integer_to_binary(MaxFileSize),
-    MaxSizeEl = #xmlel{name = <<"max-file-size">>, children = [exml:escape_cdata(MaxFileSizeBin)]},
+    MaxSizeEl = #xmlel{name = <<"max-file-size">>,
+                       children = [#xmlcdata{content = MaxFileSizeBin}]},
     FileTooLargeEl = #xmlel{name = <<"file-too-large">>,
                             attrs = [{<<"xmlns">>, Namespace}],
                             children = [MaxSizeEl]},
@@ -248,17 +249,17 @@ get_disco_info_form(Namespace, MaxFileSizeBin) ->
 -spec header_to_xmlel({Key :: binary(), Value :: binary()}) -> exml:element().
 header_to_xmlel({Key, Value}) ->
     #xmlel{name = <<"header">>,
-           attrs = [{<<"name">>, exml:escape_attr(Key)}],
-           children = [exml:escape_cdata(Value)]}.
+           attrs = [{<<"name">>, Key}],
+           children = [#xmlcdata{content = Value}]}.
 
 
 -spec create_url_xmlel(Name :: binary(), Url :: binary(), Headers :: #{binary() => binary()},
                        Namespace :: binary()) -> exml:element().
 create_url_xmlel(Name, Url, _Headers, ?NS_HTTP_UPLOAD_025) ->
-    #xmlel{name = Name, children = [exml:escape_cdata(Url)]};
+    #xmlel{name = Name, children = [#xmlcdata{content = Url}]};
 create_url_xmlel(Name, Url, Headers, ?NS_HTTP_UPLOAD_030) ->
     HeadersXml = [header_to_xmlel(H) || H <- maps:to_list(Headers)],
-    #xmlel{name = Name, attrs = [{<<"url">>, exml:escape_attr(Url)}], children = HeadersXml}.
+    #xmlel{name = Name, attrs = [{<<"url">>, Url}], children = HeadersXml}.
 
 
 -spec is_nonempty_binary(term()) -> boolean().

test.disabled/ejabberd_tests/tests/mod_http_upload_SUITE.erl

@@ -41,7 +41,9 @@ groups() ->
                                rejects_empty_filename,
                                rejects_negative_filesize,
                                rejects_invalid_size_type,
-                               denies_slots_over_max_file_size
+                               denies_slots_over_max_file_size,
+                               sends_different_put_and_get_urls,
+                               escapes_urls_once
                               ]}].
 
 suite() ->
@@ -219,6 +221,28 @@ denies_slots_over_max_file_size(Config) ->
                                                     cdata])
       end).
 
+sends_different_put_and_get_urls(Config) ->
+    namespaced_story(
+     Config, [{bob, 1}],
+     fun(Namespace, Bob) ->
+             ServJID = upload_service(Bob),
+             Request = create_slot_request_stanza(ServJID, <<"filename.jpg">>, 123,
+                                                  undefined, Namespace),
+             Result = escalus:send_and_wait(Bob, Request),
+             escalus:assert(fun urls_not_equal/2, [Namespace], Result)
+     end).
+
+escapes_urls_once(Config) ->
+    namespaced_story(
+      Config, [{bob, 1}],
+      fun(Namespace, Bob) ->
+              ServJID = upload_service(Bob),
+              Request = create_slot_request_stanza(ServJID, <<"filename.jpg">>, 123,
+                                                   undefined, Namespace),
+              Result = escalus:send_and_wait(Bob, Request),
+              escalus:assert(fun url_contains/4, [<<"put">>, <<"&x-amz-acl=public-read">>, Namespace], Result)
+      end).
+
 %%--------------------------------------------------------------------
 %% Test helpers
 %%--------------------------------------------------------------------
@@ -286,6 +310,11 @@ url_contains(UrlType, Filename, Namespace, Result) ->
     Url = extract_url(Result, UrlType, Namespace),
     binary:match(Url, Filename) =/= nomatch.
 
+urls_not_equal(Namespace, Result) ->
+    Get = extract_url(Result, <<"get">>, Namespace),
+    Put = extract_url(Result, <<"put">>, Namespace),
+    Get =/= Put.
+
 reverse(List) when is_list(List) ->
     list_to_binary(lists:reverse(List));
 reverse(Binary) ->