Skip to content

Commit

Permalink
Fix bug with parsing negative IDs in query list.
Browse files Browse the repository at this point in the history
  • Loading branch information
zerebubuth committed Jul 19, 2016
1 parent cdba3ff commit 85526d7
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 2 deletions.
17 changes: 15 additions & 2 deletions src/api06/handler_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,21 @@ vector<osm_nwr_id_t> parse_id_list_params(request &req, const string &param_name
try {
for (vector<string>::iterator itr = strs.begin(); itr != strs.end();
++itr) {
osm_nwr_id_t id = lexical_cast<osm_nwr_id_t>(*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<int64_t>(*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
Expand Down
8 changes: 8 additions & 0 deletions test/node.testcore/nodes_negative.case
Original file line number Diff line number Diff line change
@@ -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
---
21 changes: 21 additions & 0 deletions test/test_parse_id_list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,32 @@ void test_parse_returns_no_duplicates() {
}
}

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<osm_nwr_id_t> 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<int64_t>::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;
Expand Down

0 comments on commit 85526d7

Please sign in to comment.