diff --git a/core/include/irods/private/http_api/common.hpp b/core/include/irods/private/http_api/common.hpp index 71645e79..afe40514 100644 --- a/core/include/irods/private/http_api/common.hpp +++ b/core/include/irods/private/http_api/common.hpp @@ -151,8 +151,8 @@ namespace irods::http auto encode(std::string_view _to_encode) -> std::string; - // TODO Create a better name. - auto to_argument_list(const std::string_view _urlencoded_string) -> std::unordered_map; + auto parse_urlencoded_data(const std::string_view _urlencoded_string) + -> std::unordered_map; auto get_url_path(const std::string& _url) -> std::optional; diff --git a/core/src/common.cpp b/core/src/common.cpp index 6a3aad3e..cdd632e5 100644 --- a/core/src/common.cpp +++ b/core/src/common.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include @@ -116,8 +117,8 @@ namespace irods::http return encoded_data; } // encode - // TODO Create a better name. - auto to_argument_list(const std::string_view _urlencoded_string) -> std::unordered_map + auto parse_urlencoded_data(const std::string_view _urlencoded_string) + -> std::unordered_map { if (_urlencoded_string.empty()) { return {}; @@ -128,25 +129,37 @@ namespace irods::http std::vector tokens; boost::split(tokens, _urlencoded_string, boost::is_any_of("&")); - std::vector kvp; - for (auto&& t : tokens) { - boost::split(kvp, t, boost::is_any_of("=")); + if (t.empty()) { + continue; + } - if (kvp.size() == 2) { - auto value = decode(kvp[1]); - boost::replace_all(value, "+", " "); - kvps.insert_or_assign(std::move(kvp[0]), value); + std::string name; + std::string value; + + if (const auto pos = t.find('='); pos != std::string::npos) { + name = t.substr(0, pos); + value = t.substr(pos + 1); + } + else { + name = t; + } + + if (!name.empty()) { + boost::replace_all(name, "+", " "); + name = decode(name); } - else if (kvp.size() == 1) { - kvps.insert_or_assign(std::move(kvp[0]), ""); + + if (!value.empty()) { + boost::replace_all(value, "+", " "); + value = decode(value); } - kvp.clear(); + kvps.insert_or_assign(std::move(name), std::move(value)); } return kvps; - } // to_argument_list + } // parse_urlencoded_data auto get_url_path(const std::string& _url) -> std::optional { @@ -180,65 +193,36 @@ namespace irods::http return std::nullopt; } // get_url_path - auto parse_url(const std::string& _url) -> url + auto parse_url(const request_type& _req) -> url { namespace logging = irods::http::log; - std::unique_ptr curl{curl_url(), curl_url_cleanup}; - - if (!curl) { - logging::error("{}: Could not initialize CURLU handle.", __func__); - THROW(SYS_LIBRARY_ERROR, "curl_url error."); - } - - // Include a bogus prefix. We only care about the path and query parts of the URL. - if (const auto ec = curl_url_set(curl.get(), CURLUPART_URL, _url.c_str(), 0); ec) { - logging::error("{}: curl_url_set error: {}", __func__, ec); - THROW(SYS_LIBRARY_ERROR, "curl_url_set(CURLUPART_URL) error."); - } - url url; - using curl_string = std::unique_ptr; - - // Extract the path. - // This is what we use to route requests to the various endpoints. - char* path{}; - if (const auto ec = curl_url_get(curl.get(), CURLUPART_PATH, &path, 0); ec == 0) { - curl_string cpath{path, curl_free}; - if (path) { - url.path = path; - } - } - else { - logging::error("{}: curl_url_get(CURLUPART_PATH) error: {}", __func__, ec); - THROW(SYS_LIBRARY_ERROR, "curl_url_get(CURLUPART_PATH) error."); + auto parse_result = boost::urls::parse_origin_form(_req.target()); + if (!parse_result) { + const auto& ec = parse_result.error(); + logging::error( + "{}: Could not parse URL path [{}]; error code=[{}], message=[{}].", + __func__, + _req.target(), + ec.value(), + ec.message()); + THROW(SYS_LIBRARY_ERROR, "URL parse error"); } - // Extract the query. - // ChatGPT states that the values in the key value pairs must escape embedded equal signs. - // This allows the HTTP server to parse the query string correctly. Therefore, we don't have - // to protect against that case. The client must send the correct URL escaped input. - char* query{}; - if (const auto ec = curl_url_get(curl.get(), CURLUPART_QUERY, &query, 0); ec == 0) { - curl_string cs{query, curl_free}; - if (query) { - url.query = to_argument_list(query); - } - } - else { - logging::error("{}: curl_url_get(CURLUPART_QUERY) error: {}", __func__, ec); - THROW(SYS_LIBRARY_ERROR, "curl_url_get(CURLUPART_QUERY) error."); + // Extract the path. This is what we use to route requests to the various endpoints. + auto uv = *parse_result; + url.path = uv.encoded_path(); + + // Extract the query and create a mapping of key-value pairs. + for (auto&& p : uv.params()) { + url.query.insert_or_assign(p.key, p.value); } return url; } // parse_url - auto parse_url(const request_type& _req) -> url - { - return parse_url(fmt::format("http://ignored{}", _req.target())); - } // parse_url - auto url_encode_body(const body_arguments& _args) -> std::string { auto encode_pair{[](const body_arguments::value_type& i) { @@ -466,7 +450,7 @@ namespace irods::http args = irods::http::parse_multipart_form_data(*boundary, _req.body()); } else if (boost::istarts_with(content_type, "application/x-www-form-urlencoded")) { - args = irods::http::to_argument_list(_req.body()); + args = irods::http::parse_urlencoded_data(_req.body()); } else { logging::error("{}: Content type [{}] not supported.", __func__, content_type); diff --git a/test/test_irods_http_api.py b/test/test_irods_http_api.py index 62b4643d..ac098261 100644 --- a/test/test_irods_http_api.py +++ b/test/test_irods_http_api.py @@ -733,6 +733,46 @@ def test_server_reports_error_when_http_method_is_not_supported(self): def test_server_reports_error_when_op_is_not_supported(self): do_test_server_reports_error_when_op_is_not_supported(self) + def test_server_preserves_plus_sign_in_application_x_www_form_urlencoded_content(self): + rodsuser_headers = {'Authorization': f'Bearer {self.rodsuser_bearer_token}'} + collection = f'/{self.zone_name}/home/{self.rodsuser_username}/=issue+478' + + try: + # Create a collection that has a "+" in its logical path. + r = requests.post( + self.url_endpoint, + headers={ + 'Authorization': rodsuser_headers['Authorization'], + 'Content-Type': 'application/x-www-form-urlencoded' + }, + data={ + 'op': 'create', + 'lpath': collection + } + ) + self.logger.debug(r.content) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.json()['irods_response']['status_code'], 0) + + # Show the "+" is preserved and NOT converted to a space (" "). + r = requests.get(f'{self.url_base}/query', headers=rodsuser_headers, params={ + 'op': 'execute_genquery', + 'query': f"select COLL_NAME where COLL_NAME = '{collection}'" + }) + self.logger.debug(r.content) + self.assertEqual(r.status_code, 200) + result = r.json() + self.assertEqual(result['irods_response']['status_code'], 0) + self.assertEqual(len(result['rows']), 1) + self.assertEqual(result['rows'][0][0], collection) + + finally: + r = requests.post(self.url_endpoint, headers=rodsuser_headers, data={ + 'op': 'remove', + 'lpath': collection + }) + self.logger.debug(r.content) + @unittest.skip('Test needs to be implemented.') def test_return_error_on_missing_parameters(self): pass