Skip to content

Commit 409528f

Browse files
committed
Apply a global radius limit when bearing filtering is in use.
This should effectively prevent traversing the whole RTree accidentally when a highly specific bearing is supplied. The assumption is that anyone who can supply a bearing should also already have a pretty good idea of the coordinate they want.
1 parent 0a6ef58 commit 409528f

18 files changed

+167
-129
lines changed

include/engine/engine_config.hpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ namespace engine
5353
* - Match
5454
* - Nearest
5555
*
56+
* You can restrict the coordinate search radius when a bearing is supplied. This helps
57+
* prevent the rtree search expanding if a very narrow bearing filter is supplied.
58+
*
5659
* In addition, shared memory can be used for datasets loaded with osrm-datastore.
5760
*
5861
* \see OSRM, StorageConfig
@@ -61,13 +64,16 @@ struct EngineConfig final
6164
{
6265
bool IsValid() const;
6366

67+
constexpr static const int UNLIMITED = -1;
68+
6469
storage::StorageConfig storage_config;
65-
int max_locations_trip = -1;
66-
int max_locations_viaroute = -1;
67-
int max_locations_distance_table = -1;
68-
int max_locations_map_matching = -1;
69-
int max_results_nearest = -1;
70-
int max_radius_nearest = -1; // in meters
70+
int max_locations_trip = UNLIMITED;
71+
int max_locations_viaroute = UNLIMITED;
72+
int max_locations_distance_table = UNLIMITED;
73+
int max_locations_map_matching = UNLIMITED;
74+
int max_results_nearest = UNLIMITED;
75+
76+
double max_radius_when_bearings = UNLIMITED; // in meters
7177

7278
bool use_shared_memory = true;
7379
};

include/engine/plugins/match.hpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ class MatchPlugin : public BasePlugin
2727
static const constexpr double DEFAULT_GPS_PRECISION = 5;
2828
static const constexpr double RADIUS_MULTIPLIER = 3;
2929

30-
MatchPlugin(datafacade::BaseDataFacade &facade_, const int max_locations_map_matching)
31-
: BasePlugin(facade_), map_matching(&facade_, heaps, DEFAULT_GPS_PRECISION),
30+
MatchPlugin(datafacade::BaseDataFacade &facade_, const int max_locations_map_matching,
31+
const double max_radius_when_bearings_)
32+
: BasePlugin(facade_, max_radius_when_bearings_), map_matching(&facade_, heaps, DEFAULT_GPS_PRECISION),
3233
shortest_path(&facade_, heaps), max_locations_map_matching(max_locations_map_matching)
3334
{
3435
}

include/engine/plugins/nearest.hpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,13 @@ class NearestPlugin final : public BasePlugin
1616
{
1717
public:
1818
explicit NearestPlugin(datafacade::BaseDataFacade &facade,
19-
const int max_results,
20-
const int max_radius);
19+
const int max_results_nearest,
20+
const double max_radius_when_bearings);
2121

2222
Status HandleRequest(const api::NearestParameters &params, util::json::Object &result);
2323

2424
private:
2525
const int max_results;
26-
const int max_radius;
2726
};
2827
}
2928
}

include/engine/plugins/plugin_base.hpp

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "engine/datafacade/datafacade_base.hpp"
66
#include "engine/phantom_node.hpp"
77
#include "engine/status.hpp"
8+
#include "engine/engine_config.hpp"
89

910
#include "util/coordinate.hpp"
1011
#include "util/coordinate_calculation.hpp"
@@ -27,7 +28,8 @@ class BasePlugin
2728
{
2829
protected:
2930
datafacade::BaseDataFacade &facade;
30-
BasePlugin(datafacade::BaseDataFacade &facade_) : facade(facade_) {}
31+
const double max_radius_when_bearings;
32+
BasePlugin(datafacade::BaseDataFacade &facade_, const double max_radius_when_bearings_) : facade(facade_), max_radius_when_bearings(max_radius_when_bearings_) {}
3133

3234
bool CheckAllCoordinates(const std::vector<util::Coordinate> &coordinates)
3335
{
@@ -37,6 +39,19 @@ class BasePlugin
3739
});
3840
}
3941

42+
bool CheckAllRadiuses(const api::BaseParameters &parameters)
43+
{
44+
if (parameters.bearings.empty()) return true;
45+
46+
if (max_radius_when_bearings == EngineConfig::UNLIMITED) return true;
47+
48+
return !std::any_of(
49+
std::begin(parameters.radiuses), std::end(parameters.radiuses),
50+
[this](const auto &radius) {
51+
return radius && *radius > max_radius_when_bearings;
52+
});
53+
}
54+
4055
Status Error(const std::string &code,
4156
const std::string &message,
4257
util::json::Object &json_result) const
@@ -137,7 +152,7 @@ class BasePlugin
137152
{
138153
phantom_nodes[i] =
139154
facade.NearestPhantomNodesInRange(parameters.coordinates[i],
140-
radiuses[i],
155+
max_radius_when_bearings != EngineConfig::UNLIMITED ? std::min(radiuses[i], max_radius_when_bearings) : radiuses[i],
141156
parameters.bearings[i]->bearing,
142157
parameters.bearings[i]->range);
143158
}
@@ -181,16 +196,27 @@ class BasePlugin
181196
{
182197
phantom_nodes[i] = facade.NearestPhantomNodes(parameters.coordinates[i],
183198
number_of_results,
184-
*parameters.radiuses[i],
199+
max_radius_when_bearings != EngineConfig::UNLIMITED ? std::min(*parameters.radiuses[i],max_radius_when_bearings) : *parameters.radiuses[i],
185200
parameters.bearings[i]->bearing,
186201
parameters.bearings[i]->range);
187202
}
188203
else
189204
{
190-
phantom_nodes[i] = facade.NearestPhantomNodes(parameters.coordinates[i],
191-
number_of_results,
192-
parameters.bearings[i]->bearing,
193-
parameters.bearings[i]->range);
205+
if (max_radius_when_bearings != EngineConfig::UNLIMITED)
206+
{
207+
phantom_nodes[i] = facade.NearestPhantomNodes(parameters.coordinates[i],
208+
number_of_results,
209+
parameters.bearings[i]->bearing,
210+
parameters.bearings[i]->range);
211+
}
212+
else
213+
{
214+
phantom_nodes[i] = facade.NearestPhantomNodes(parameters.coordinates[i],
215+
number_of_results,
216+
max_radius_when_bearings,
217+
parameters.bearings[i]->bearing,
218+
parameters.bearings[i]->range);
219+
}
194220
}
195221
}
196222
else
@@ -242,17 +268,29 @@ class BasePlugin
242268
phantom_node_pairs[i] =
243269
facade.NearestPhantomNodeWithAlternativeFromBigComponent(
244270
parameters.coordinates[i],
245-
*parameters.radiuses[i],
271+
max_radius_when_bearings != EngineConfig::UNLIMITED ? std::min(*parameters.radiuses[i],max_radius_when_bearings) : *parameters.radiuses[i],
246272
parameters.bearings[i]->bearing,
247273
parameters.bearings[i]->range);
248274
}
249275
else
250276
{
251-
phantom_node_pairs[i] =
252-
facade.NearestPhantomNodeWithAlternativeFromBigComponent(
253-
parameters.coordinates[i],
254-
parameters.bearings[i]->bearing,
255-
parameters.bearings[i]->range);
277+
if (max_radius_when_bearings == EngineConfig::UNLIMITED)
278+
{
279+
phantom_node_pairs[i] =
280+
facade.NearestPhantomNodeWithAlternativeFromBigComponent(
281+
parameters.coordinates[i],
282+
parameters.bearings[i]->bearing,
283+
parameters.bearings[i]->range);
284+
}
285+
else
286+
{
287+
phantom_node_pairs[i] =
288+
facade.NearestPhantomNodeWithAlternativeFromBigComponent(
289+
parameters.coordinates[i],
290+
max_radius_when_bearings,
291+
parameters.bearings[i]->bearing,
292+
parameters.bearings[i]->range);
293+
}
256294
}
257295
}
258296
else

include/engine/plugins/table.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ class TablePlugin final : public BasePlugin
1919
{
2020
public:
2121
explicit TablePlugin(datafacade::BaseDataFacade &facade,
22-
const int max_locations_distance_table);
22+
const int max_locations_distance_table,
23+
const double max_radius_when_bearings);
2324

2425
Status HandleRequest(const api::TableParameters &params, util::json::Object &result);
2526

include/engine/plugins/tile.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ namespace plugins
2424
class TilePlugin final : public BasePlugin
2525
{
2626
public:
27-
TilePlugin(datafacade::BaseDataFacade &facade) : BasePlugin(facade) {}
27+
TilePlugin(datafacade::BaseDataFacade &facade) : BasePlugin(facade, EngineConfig::UNLIMITED) {}
2828

2929
Status HandleRequest(const api::TileParameters &parameters, std::string &pbf_buffer);
3030
};

include/engine/plugins/trip.hpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,11 @@ class TripPlugin final : public BasePlugin
3838
const std::vector<NodeID> &trip);
3939

4040
public:
41-
explicit TripPlugin(datafacade::BaseDataFacade &facade_, const int max_locations_trip_)
42-
: BasePlugin(facade_), shortest_path(&facade_, heaps), duration_table(&facade_, heaps),
43-
max_locations_trip(max_locations_trip_)
41+
explicit TripPlugin(datafacade::BaseDataFacade &facade_,
42+
const int max_locations_trip_,
43+
const double max_radius_when_bearings_)
44+
: BasePlugin(facade_, max_radius_when_bearings_), shortest_path(&facade_, heaps),
45+
duration_table(&facade_, heaps), max_locations_trip(max_locations_trip_)
4446
{
4547
}
4648

include/engine/plugins/viaroute.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,12 @@ class ViaRoutePlugin final : public BasePlugin
3232
routing_algorithms::ShortestPathRouting<datafacade::BaseDataFacade> shortest_path;
3333
routing_algorithms::AlternativeRouting<datafacade::BaseDataFacade> alternative_path;
3434
routing_algorithms::DirectShortestPathRouting<datafacade::BaseDataFacade> direct_shortest_path;
35-
int max_locations_viaroute;
35+
const int max_locations_viaroute;
3636

3737
public:
38-
explicit ViaRoutePlugin(datafacade::BaseDataFacade &facade, int max_locations_viaroute);
38+
explicit ViaRoutePlugin(datafacade::BaseDataFacade &facade,
39+
const int max_locations_viaroute,
40+
const double max_radius_when_bearings);
3941

4042
Status HandleRequest(const api::RouteParameters &route_parameters,
4143
util::json::Object &json_result);

src/engine/engine.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,11 @@ Engine::Engine(EngineConfig &config)
144144
// Register plugins
145145
using namespace plugins;
146146

147-
route_plugin = create<ViaRoutePlugin>(*query_data_facade, config.max_locations_viaroute);
148-
table_plugin = create<TablePlugin>(*query_data_facade, config.max_locations_distance_table);
149-
nearest_plugin = create<NearestPlugin>(
150-
*query_data_facade, config.max_results_nearest, config.max_radius_nearest);
151-
trip_plugin = create<TripPlugin>(*query_data_facade, config.max_locations_trip);
152-
match_plugin = create<MatchPlugin>(*query_data_facade, config.max_locations_map_matching);
147+
route_plugin = create<ViaRoutePlugin>(*query_data_facade, config.max_locations_viaroute, config.max_radius_when_bearings);
148+
table_plugin = create<TablePlugin>(*query_data_facade, config.max_locations_distance_table, config.max_radius_when_bearings);
149+
nearest_plugin = create<NearestPlugin>(*query_data_facade, config.max_results_nearest, config.max_radius_when_bearings);
150+
trip_plugin = create<TripPlugin>(*query_data_facade, config.max_locations_trip, config.max_radius_when_bearings);
151+
match_plugin = create<MatchPlugin>(*query_data_facade, config.max_locations_map_matching, config.max_radius_when_bearings);
153152
tile_plugin = create<TilePlugin>(*query_data_facade);
154153
}
155154

src/engine/engine_config.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ bool EngineConfig::IsValid() const
2424
unlimited_or_more_than(max_locations_map_matching, 2) &&
2525
unlimited_or_more_than(max_locations_trip, 2) &&
2626
unlimited_or_more_than(max_locations_viaroute, 2) &&
27-
unlimited_or_more_than(max_results_nearest, 1) &&
28-
unlimited_or_more_than(max_radius_nearest, 1);
27+
unlimited_or_more_than(max_results_nearest, 0) &&
28+
unlimited_or_more_than(max_radius_when_bearings, 0);
2929

3030
return ((use_shared_memory && all_path_are_empty) || storage_config.IsValid()) && limits_valid;
3131
}

src/engine/plugins/match.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@ Status MatchPlugin::HandleRequest(const api::MatchParameters &parameters,
123123
return Error("InvalidValue", "Invalid coordinate value.", json_result);
124124
}
125125

126+
if (!CheckAllRadiuses(parameters))
127+
{
128+
return Error("TooBig", "When using a bearing filter, the maximum search radius is limited to " + std::to_string(max_radius_when_bearings) + "m", json_result);
129+
}
130+
126131
// assuming radius is the standard deviation of a normal distribution
127132
// that models GPS noise (in this model), x3 should give us the correct
128133
// search radius with > 99% confidence

src/engine/plugins/nearest.cpp

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ namespace plugins
1919

2020
NearestPlugin::NearestPlugin(datafacade::BaseDataFacade &facade,
2121
const int max_results_,
22-
const int max_radius_)
23-
: BasePlugin{facade}, max_results{max_results_}, max_radius{max_radius_}
22+
const double max_radius_when_bearing_)
23+
: BasePlugin{facade, max_radius_when_bearing_}, max_results{max_results_}
2424
{
2525
}
2626

@@ -37,37 +37,25 @@ Status NearestPlugin::HandleRequest(const api::NearestParameters &params,
3737
return Error("InvalidOptions", "Only one input coordinate is supported", json_result);
3838
}
3939

40-
// Artificially set and / or limit the params.radiuses array for GetPhantomNodes.
41-
// This sets a limit on nearest queries with bearing and large number of results.
42-
43-
// params are const (read-only) but we have to constrain the radiuses afterwards.
44-
// We want to keep the user-interface as is, so make a copy. Should be cheap.
45-
auto constraint_params = params;
46-
47-
if (max_radius != -1)
40+
if (max_results != -1 && params.number_of_results > boost::numeric_cast<unsigned>(max_results))
4841
{
49-
const auto max_radius_m = static_cast<double>(max_radius);
50-
51-
if (constraint_params.radiuses.empty())
52-
constraint_params.radiuses.push_back(boost::make_optional(max_radius_m));
53-
else if (constraint_params.radiuses[0])
54-
boost::algorithm::clamp(*constraint_params.radiuses[0], 0., max_radius_m);
42+
return Error("TooBig", "Too many results requested, maximum is " + std::to_string(max_results), json_result);
5543
}
5644

57-
if (max_results != -1)
45+
if (!CheckAllRadiuses(params))
5846
{
59-
constraint_params.number_of_results = boost::algorithm::clamp(params.number_of_results, 0, max_results);
47+
return Error("TooBig", "When using a bearing filter, the maximum search radius is limited to " + std::to_string(max_radius_when_bearings) + "m", json_result);
6048
}
6149

62-
auto phantom_nodes = GetPhantomNodes(constraint_params, constraint_params.number_of_results);
50+
auto phantom_nodes = GetPhantomNodes(params, params.number_of_results);
6351

6452
if (phantom_nodes.front().size() == 0)
6553
{
6654
return Error("NoSegment", "Could not find a matching segments for coordinate", json_result);
6755
}
6856
BOOST_ASSERT(phantom_nodes.front().size() > 0);
6957

70-
api::NearestAPI nearest_api(facade, constraint_params);
58+
api::NearestAPI nearest_api(facade, params);
7159
nearest_api.MakeResponse(phantom_nodes, json_result);
7260

7361
return Status::Ok;

src/engine/plugins/table.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ namespace engine
2323
namespace plugins
2424
{
2525

26-
TablePlugin::TablePlugin(datafacade::BaseDataFacade &facade, const int max_locations_distance_table)
27-
: BasePlugin{facade}, distance_table(&facade, heaps),
26+
TablePlugin::TablePlugin(datafacade::BaseDataFacade &facade, const int max_locations_distance_table, const double max_radius_when_bearings_)
27+
: BasePlugin{facade, max_radius_when_bearings_}, distance_table(&facade, heaps),
2828
max_locations_distance_table(max_locations_distance_table)
2929
{
3030
}
@@ -58,6 +58,11 @@ Status TablePlugin::HandleRequest(const api::TableParameters &params, util::json
5858
return Error("TooBig", "Too many table coordinates", result);
5959
}
6060

61+
if (!CheckAllRadiuses(params))
62+
{
63+
return Error("TooBig", "When using a bearing filter, the maximum search radius is limited to " + std::to_string(max_radius_when_bearings) + "m", result);
64+
}
65+
6166
auto snapped_phantoms = SnapPhantomNodes(GetPhantomNodes(params));
6267
auto result_table = distance_table(snapped_phantoms, params.sources, params.destinations);
6368

src/engine/plugins/trip.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,11 @@ Status TripPlugin::HandleRequest(const api::TripParameters &parameters,
157157
return Error("InvalidValue", "Invalid coordinate value.", json_result);
158158
}
159159

160+
if (!CheckAllRadiuses(parameters))
161+
{
162+
return Error("TooBig", "When using a bearing filter, the maximum search radius is limited to " + std::to_string(max_radius_when_bearings) + "m", json_result);
163+
}
164+
160165
auto phantom_node_pairs = GetPhantomNodes(parameters);
161166
if (phantom_node_pairs.size() != parameters.coordinates.size())
162167
{

src/engine/plugins/viaroute.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ namespace engine
2121
namespace plugins
2222
{
2323

24-
ViaRoutePlugin::ViaRoutePlugin(datafacade::BaseDataFacade &facade_, int max_locations_viaroute)
25-
: BasePlugin(facade_), shortest_path(&facade_, heaps), alternative_path(&facade_, heaps),
24+
ViaRoutePlugin::ViaRoutePlugin(datafacade::BaseDataFacade &facade_, const int max_locations_viaroute, const double max_radius_when_bearings_)
25+
: BasePlugin(facade_, max_radius_when_bearings_), shortest_path(&facade_, heaps), alternative_path(&facade_, heaps),
2626
direct_shortest_path(&facade_, heaps), max_locations_viaroute(max_locations_viaroute)
2727
{
2828
}
@@ -47,6 +47,11 @@ Status ViaRoutePlugin::HandleRequest(const api::RouteParameters &route_parameter
4747
return Error("InvalidValue", "Invalid coordinate value.", json_result);
4848
}
4949

50+
if (!CheckAllRadiuses(route_parameters))
51+
{
52+
return Error("TooBig", "When using a bearing filter, the maximum search radius is limited to " + std::to_string(max_radius_when_bearings) + "m", json_result);
53+
}
54+
5055
auto phantom_node_pairs = GetPhantomNodes(route_parameters);
5156
if (phantom_node_pairs.size() != route_parameters.coordinates.size())
5257
{

0 commit comments

Comments
 (0)