diff --git a/src/api06/handler_utils.cpp b/src/api06/handler_utils.cpp index c61a1499e..3c42da0c4 100644 --- a/src/api06/handler_utils.cpp +++ b/src/api06/handler_utils.cpp @@ -45,8 +45,21 @@ vector parse_id_list_params(request &req, const string ¶m_name try { for (vector::iterator itr = strs.begin(); itr != strs.end(); ++itr) { - osm_nwr_id_t id = lexical_cast(*itr); - myids.push_back(id); + // parse as a 64-bit signed int to catch negative numbers. postgres + // bigint is int64_t anyway, so this doesn't truncate the allowed + // range. + int64_t id = lexical_cast(*itr); + // note: osm_nwr_id_t is currently uint64_t, so int64_t cannot + // overflow it in the positive direction. this would need to change + // if the types changed. + if (id > 0) { + myids.push_back(osm_nwr_id_t(id)); + + } else { + // simulate rails_port behaviour - things that we can't parse are + // treated as zero. + myids.push_back(0); + } } } catch (const bad_lexical_cast &) { // note: this is pretty silly, and may change when the rails_port diff --git a/test/node.testcore/nodes_negative.case b/test/node.testcore/nodes_negative.case new file mode 100644 index 000000000..90ba6b693 --- /dev/null +++ b/test/node.testcore/nodes_negative.case @@ -0,0 +1,8 @@ +# tests that passing a list of nodes, some negative, will result in +# a 404 response. negative nodes do not appear in the database, so +# any query containing them will fail. +Request-Method: GET +Request-URI: /api/0.6/nodes?nodes=-1875196430,1970729486,-715595887,153329585,276538320,276538320,276538320,276538320,557671215,268800768,268800768,272134694,416571249,4118507737,639141976,-120408340,4118507737,4118507737,-176459559,-176459559,-176459559,416571249,-176459559,-176459559,-176459559,557671215 +--- +Status: 404 Not Found +--- diff --git a/test/test_parse_id_list.cpp b/test/test_parse_id_list.cpp index ee656ceeb..fd44fff02 100644 --- a/test/test_parse_id_list.cpp +++ b/test/test_parse_id_list.cpp @@ -1,6 +1,7 @@ #include "cgimap/api06/handler_utils.hpp" #include #include +#include #include #include @@ -40,26 +41,68 @@ struct test_request : public request { std::map m_params; }; -} // anonymous namespace - -int main(int argc, char *argv[]) { - std::string query_str = "nodes=1,1,1,1"; +std::vector parse_query_str(std::string query_str) { test_request req; req.set_header("REQUEST_METHOD", "GET"); req.set_header("QUERY_STRING", query_str); req.set_header("PATH_INFO", "/api/0.6/nodes"); + return api06::parse_id_list_params(req, "nodes"); +} + +void test_parse_returns_no_duplicates() { + std::string query_str = "nodes=1,1,1,1"; + // the container returned from parse_id_list_params should not contain // any duplicates. - std::vector ids = api06::parse_id_list_params(req, "nodes"); + std::vector ids = parse_query_str(query_str); + if (ids.size() != 1) { - std::cerr << "Parsing " << query_str << " as a list of nodes should " - << "discard duplicates, but got: {"; + std::ostringstream err; + err << "Parsing " << query_str << " as a list of nodes should " + << "discard duplicates, but got: {"; BOOST_FOREACH(osm_nwr_id_t id, ids) { - std::cerr << id << ", "; + err << id << ", "; } - std::cerr << "}\n"; + err << "}\n"; + throw std::runtime_error(err.str()); + } +} + +void test_parse_negative_nodes() { + std::string query_str = + "nodes=-1875196430,1970729486,-715595887,153329585,276538320,276538320," + "276538320,276538320,557671215,268800768,268800768,272134694,416571249," + "4118507737,639141976,-120408340,4118507737,4118507737,-176459559," + "-176459559,-176459559,416571249,-176459559,-176459559,-176459559," + "557671215"; + + std::vector ids = parse_query_str(query_str); + + // maximum ID that postgres can handle is 2^63-1, so that should never + // be returned by the parsing function. + const osm_nwr_id_t max_id = std::numeric_limits::max(); + BOOST_FOREACH(osm_nwr_id_t id, ids) { + if (id > max_id) { + throw std::runtime_error("Found ID > max allowed ID in parsed list."); + } + } +} + +} // anonymous namespace + +int main(int argc, char *argv[]) { + try { + test_parse_returns_no_duplicates(); + test_parse_negative_nodes(); + + } catch (const std::exception &e) { + std::cout << "Error: " << e.what() << std::endl; return 1; + + } catch (...) { + std::cout << "Unknown exception type." << std::endl; + return 99; } return 0;