Skip to content

Commit bf44859

Browse files
mmd-osmmmd-osm
committed
Add the ability to limit changeset size
1 parent 26cd7fa commit bf44859

File tree

12 files changed

+256
-0
lines changed

12 files changed

+256
-0
lines changed

‎include/cgimap/api06/changeset_upload/changeset_updater.hpp

+2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ class Changeset_Updater {
2626

2727
virtual void update_changeset(uint32_t num_new_changes, bbox_t bbox) = 0;
2828

29+
virtual bbox_t get_bbox() const = 0;
30+
2931
virtual osm_changeset_id_t api_create_changeset(const std::map<std::string, std::string>&) = 0;
3032

3133
virtual void api_update_changeset(const std::map<std::string, std::string>&) = 0;

‎include/cgimap/backend/apidb/changeset_upload/changeset_updater.hpp

+2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ class ApiDB_Changeset_Updater : public api06::Changeset_Updater {
3030

3131
void update_changeset(const uint32_t num_new_changes, const bbox_t bbox) override;
3232

33+
bbox_t get_bbox() const override;
34+
3335
osm_changeset_id_t api_create_changeset(const std::map<std::string, std::string>&) override;
3436

3537
void api_update_changeset(const std::map<std::string, std::string>&) override;

‎include/cgimap/backend/apidb/pgsql_update.hpp

+2
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ class pgsql_update : public data_update {
4343

4444
uint32_t get_rate_limit(osm_user_id_t uid) override;
4545

46+
uint64_t get_bbox_size_limit(osm_user_id_t uid) override;
47+
4648
/**
4749
* abstracts the creation of transactions for the
4850
* data updates.

‎include/cgimap/data_update.hpp

+3
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ class data_update {
5656
// get the current rate limit for changeset uploads for a given user id
5757
virtual uint32_t get_rate_limit(osm_user_id_t) = 0;
5858

59+
// get the current maximum bounding box size for a given user id
60+
virtual uint64_t get_bbox_size_limit(osm_user_id_t uid) = 0;
61+
5962
/**
6063
* factory for the creation of data updates. this abstracts away
6164
* the creation process of transactions, and allows some up-front

‎include/cgimap/options.hpp

+14
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class global_settings_base {
3737
virtual uint32_t get_ratelimiter_ratelimit(bool) const = 0;
3838
virtual uint32_t get_ratelimiter_maxdebt(bool) const = 0;
3939
virtual bool get_ratelimiter_upload() const = 0;
40+
virtual bool get_bbox_size_limiter_upload() const = 0;
4041
};
4142

4243
class global_settings_default : public global_settings_base {
@@ -107,6 +108,10 @@ class global_settings_default : public global_settings_base {
107108
bool get_ratelimiter_upload() const override {
108109
return false;
109110
}
111+
112+
bool get_bbox_size_limiter_upload() const override {
113+
return false;
114+
}
110115
};
111116

112117
class global_settings_via_options : public global_settings_base {
@@ -193,6 +198,10 @@ class global_settings_via_options : public global_settings_base {
193198
return m_ratelimiter_upload;
194199
}
195200

201+
bool get_bbox_size_limiter_upload() const override {
202+
return m_bbox_size_limiter_upload;
203+
}
204+
196205
private:
197206
void init_fallback_values(const global_settings_base &def);
198207
void set_new_options(const po::variables_map &options);
@@ -211,6 +220,7 @@ class global_settings_via_options : public global_settings_base {
211220
void set_ratelimiter_ratelimit(const po::variables_map &options);
212221
void set_ratelimiter_maxdebt(const po::variables_map &options);
213222
void set_ratelimiter_upload(const po::variables_map &options);
223+
void set_bbox_size_limiter_upload(const po::variables_map &options);
214224
bool validate_timeout(const std::string &timeout) const;
215225

216226
uint32_t m_payload_max_size;
@@ -230,6 +240,7 @@ class global_settings_via_options : public global_settings_base {
230240
uint32_t m_ratelimiter_maxdebt;
231241
uint32_t m_moderator_ratelimiter_maxdebt;
232242
bool m_ratelimiter_upload;
243+
bool m_bbox_size_limiter_upload;
233244
};
234245

235246
class global_settings final {
@@ -284,6 +295,9 @@ class global_settings final {
284295
// Use ratelimiter for changeset uploads
285296
static bool get_ratelimiter_upload() { return settings->get_ratelimiter_upload(); }
286297

298+
// Use bbox size limiter for changeset uploads
299+
static bool get_bbox_size_limiter_upload() { return settings->get_bbox_size_limiter_upload(); }
300+
287301
private:
288302
static std::unique_ptr<global_settings_base> settings; // gets initialized with global_settings_default instance
289303
};

‎include/cgimap/util.hpp

+4
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ class bbox_t {
124124
os << "[" << bbox.minlat << "," << bbox.minlon << "," << bbox.maxlat << "," << bbox.maxlon << "]";
125125
return os;
126126
}
127+
128+
long linear_size() const {
129+
return ((maxlon - minlon) + (maxlat - minlat));
130+
}
127131
};
128132

129133
#endif

‎src/api06/changeset_upload_handler.cpp

+20
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,26 @@ changeset_upload_responder::changeset_upload_responder(mime::type mt,
7070

7171
changeset_updater->update_changeset(new_changes, handler.get_bbox());
7272

73+
if (global_settings::get_bbox_size_limiter_upload()) {
74+
75+
auto const cs_bbox = changeset_updater->get_bbox();
76+
77+
if (!(cs_bbox == bbox_t())) // valid bbox?
78+
{
79+
auto const max_bbox_size = upd.get_bbox_size_limit(*user_id);
80+
81+
if (cs_bbox.linear_size() > max_bbox_size) {
82+
83+
logger::message(
84+
fmt::format(
85+
"Upload of {} changes by user {} in changeset {} blocked due to bbox size limit exceeded, max bbox size {}",
86+
new_changes, *user_id, changeset, max_bbox_size));
87+
88+
throw http::payload_too_large("Changeset bounding box size limit exceeded.");
89+
}
90+
}
91+
}
92+
7393
upd.commit();
7494
}
7595

‎src/backend/apidb/changeset_upload/changeset_updater.cpp

+5
Original file line numberDiff line numberDiff line change
@@ -344,3 +344,8 @@ void ApiDB_Changeset_Updater::changeset_insert_cs ()
344344
changeset = r[0]["id"].as<osm_changeset_id_t> ();
345345
}
346346
}
347+
348+
bbox_t ApiDB_Changeset_Updater::get_bbox() const
349+
{
350+
return cs_bbox;
351+
}

‎src/backend/apidb/pgsql_update.cpp

+19
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,25 @@ uint32_t pgsql_update::get_rate_limit(osm_user_id_t uid)
151151
return std::max(0, rate_limit);
152152
}
153153

154+
uint64_t pgsql_update::get_bbox_size_limit(osm_user_id_t uid)
155+
{
156+
{
157+
m.prepare("api_size_limit",
158+
R"(SELECT * FROM api_size_limit($1) LIMIT 1 )");
159+
160+
auto res = m.exec_prepared("api_size_limit", uid);
161+
162+
if (res.size() != 1) {
163+
throw http::server_error("api_size_limit db function did not return any data");
164+
}
165+
166+
auto row = res[0];
167+
auto bbox_size_limit = row[0].as<int64_t>();
168+
169+
return std::max(bbox_size_limit, 0l);
170+
}
171+
}
172+
154173

155174
pgsql_update::factory::factory(const po::variables_map &opts)
156175
: m_connection(connect_db_str(opts)),

‎src/main.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ void get_options(int argc, char **argv, po::variables_map &options) {
141141
("basic_auth_support", po::value<bool>(), "enable HTTP basic authentication support")
142142
("oauth_10_support", po::value<bool>(), "enable legacy OAuth 1.0 support")
143143
("ratelimit-upload", po::value<bool>(), "enable rate limiting for changeset upload")
144+
("bbox-size-limit-upload", po::value<bool>(), "enable bbox size limit for changeset upload")
144145
;
145146
// clang-format on
146147

‎src/options.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ void global_settings_via_options::init_fallback_values(const global_settings_bas
3535
m_ratelimiter_maxdebt = def.get_ratelimiter_maxdebt(false);
3636
m_moderator_ratelimiter_maxdebt = def.get_ratelimiter_maxdebt(true);
3737
m_ratelimiter_upload = def.get_ratelimiter_upload();
38+
m_bbox_size_limiter_upload = def.get_bbox_size_limiter_upload();
3839
}
3940

4041
void global_settings_via_options::set_new_options(const po::variables_map &options) {
@@ -54,6 +55,7 @@ void global_settings_via_options::set_new_options(const po::variables_map &optio
5455
set_ratelimiter_ratelimit(options);
5556
set_ratelimiter_maxdebt(options);
5657
set_ratelimiter_upload(options);
58+
set_bbox_size_limiter_upload(options);
5759
}
5860

5961
void global_settings_via_options::set_payload_max_size(const po::variables_map &options) {
@@ -201,6 +203,11 @@ void global_settings_via_options::set_ratelimiter_upload(const po::variables_map
201203
}
202204
}
203205

206+
void global_settings_via_options::set_bbox_size_limiter_upload(const po::variables_map &options) {
207+
if (options.count("bbox-size-limit-upload")) {
208+
m_bbox_size_limiter_upload = options["bbox-size-limit-upload"].as<bool>();
209+
}
210+
}
204211

205212
bool global_settings_via_options::validate_timeout(const std::string &timeout) const {
206213
std::smatch sm;

‎test/test_apidb_backend_changeset_uploads.cpp

+177
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@ class global_settings_enable_upload_rate_limiter_test_class : public global_sett
5555
};
5656

5757

58+
class global_setting_enable_bbox_size_limiter_test_class : public global_settings_default {
59+
60+
public:
61+
// enable bbox size limiter
62+
bool get_bbox_size_limiter_upload() const override { return true; }
63+
};
64+
5865
std::unique_ptr<xmlDoc, void (*)(xmlDoc *)> getDocument(const std::string &document)
5966
{
6067
return {xmlReadDoc((xmlChar *)(document.c_str()), NULL, NULL, XML_PARSE_PEDANTIC | XML_PARSE_NONET), xmlFreeDoc};
@@ -2447,6 +2454,7 @@ TEST_CASE_METHOD( DatabaseTestsFixture, "test_osmchange_end_to_end", "[changeset
24472454

24482455
}
24492456

2457+
24502458
TEST_CASE_METHOD( DatabaseTestsFixture, "test_osmchange_rate_limiter", "[changeset][upload][db]" ) {
24512459

24522460
// Upload rate limiter enabling
@@ -2599,6 +2607,175 @@ TEST_CASE_METHOD( DatabaseTestsFixture, "test_osmchange_rate_limiter", "[changes
25992607
}
26002608
}
26012609

2610+
2611+
TEST_CASE_METHOD( DatabaseTestsFixture, "test_osmchange_bbox_size_limiter", "[changeset][upload][db]" ) {
2612+
2613+
// Upload bbox size limiter enabling
2614+
auto test_settings = std::unique_ptr<
2615+
global_setting_enable_bbox_size_limiter_test_class >(
2616+
new global_setting_enable_bbox_size_limiter_test_class());
2617+
global_settings::set_configuration(std::move(test_settings));
2618+
2619+
const std::string bearertoken = "Bearer 4f41f2328befed5a33bcabdf14483081c8df996cbafc41e313417776e8fafae8";
2620+
const std::string generator = "Test";
2621+
2622+
auto sel_factory = tdb.get_data_selection_factory();
2623+
auto upd_factory = tdb.get_data_update_factory();
2624+
2625+
null_rate_limiter limiter;
2626+
routes route;
2627+
2628+
SECTION("Initialize test data") {
2629+
2630+
tdb.run_sql(R"(
2631+
INSERT INTO users (id, email, pass_crypt, pass_salt, creation_time, display_name, data_public, status)
2632+
VALUES
2633+
(1, 'demo@example.com', 'xx', '', '2013-11-14T02:10:00Z', 'demo', true, 'confirmed');
2634+
2635+
INSERT INTO changesets (id, user_id, created_at, closed_at, num_changes)
2636+
VALUES
2637+
(1, 1, now() at time zone 'utc', now() at time zone 'utc' + '1 hour' ::interval, 0),
2638+
(3, 1, now() at time zone 'utc', now() at time zone 'utc' + '1 hour' ::interval, 0);
2639+
2640+
SELECT setval('current_nodes_id_seq', 14000000000, false);
2641+
2642+
INSERT INTO oauth_applications (id, owner_type, owner_id, name, uid, secret, redirect_uri, scopes, confidential, created_at, updated_at)
2643+
VALUES (3, 'User', 1, 'App 1', 'dHKmvGkmuoMjqhCNmTJkf-EcnA61Up34O1vOHwTSvU8', '965136b8fb8d00e2faa2faaaed99c0ec10225518d0c8d9fb1d2af701e87eb68c',
2644+
'http://demo.localhost:3000', 'write_api read_gpx', false, '2021-04-12 17:53:30', '2021-04-12 17:53:30');
2645+
2646+
INSERT INTO public.oauth_access_tokens (id, resource_owner_id, application_id, token, refresh_token, expires_in, revoked_at, created_at, scopes, previous_refresh_token)
2647+
VALUES (67, 1, 3, '4f41f2328befed5a33bcabdf14483081c8df996cbafc41e313417776e8fafae8', NULL, NULL, NULL, '2021-04-14 19:38:21', 'write_api', '');
2648+
2649+
)"
2650+
);
2651+
2652+
// Test api_size_limit database function.
2653+
// Real database function is managed outside of CGImap
2654+
2655+
tdb.run_sql(R"(
2656+
2657+
CREATE OR REPLACE FUNCTION api_size_limit(user_id bigint)
2658+
RETURNS bigint
2659+
AS $$
2660+
BEGIN
2661+
RETURN 5000000;
2662+
END;
2663+
$$ LANGUAGE plpgsql STABLE;
2664+
2665+
)");
2666+
}
2667+
2668+
SECTION("Try to upload one way with two nodes, with very large bbox")
2669+
{
2670+
// set up request headers from test case
2671+
test_request req;
2672+
req.set_header("REQUEST_METHOD", "POST");
2673+
req.set_header("REQUEST_URI", "/api/0.6/changeset/1/upload");
2674+
req.set_header("HTTP_AUTHORIZATION", bearertoken);
2675+
req.set_header("REMOTE_ADDR", "127.0.0.1");
2676+
2677+
req.set_payload(R"(<?xml version="1.0" encoding="UTF-8"?>
2678+
<osmChange version="0.6" generator="iD">
2679+
<create>
2680+
<node id='-25355' lat='68.13898255618' lon='-105.8206640625' changeset="1" />
2681+
<node id='-25357' lat='-34.30685345531' lon='80.8590234375' changeset="1" />
2682+
<way id='-579' changeset="1">
2683+
<nd ref='-25355' />
2684+
<nd ref='-25357' />
2685+
</way>
2686+
</create>
2687+
</osmChange>)" );
2688+
2689+
// execute the request
2690+
process_request(req, limiter, generator, route, *sel_factory, upd_factory.get());
2691+
2692+
CAPTURE(req.body().str());
2693+
REQUIRE(req.response_status() == 413);
2694+
}
2695+
2696+
SECTION("Try to upload twice in same changeset, two nodes with very large bbox")
2697+
{
2698+
// set up request headers from test case
2699+
{
2700+
test_request req;
2701+
req.set_header("REQUEST_METHOD", "POST");
2702+
req.set_header("REQUEST_URI", "/api/0.6/changeset/3/upload");
2703+
req.set_header("HTTP_AUTHORIZATION", bearertoken);
2704+
req.set_header("REMOTE_ADDR", "127.0.0.1");
2705+
2706+
req.set_payload(R"(<?xml version="1.0" encoding="UTF-8"?>
2707+
<osmChange version="0.6" generator="iD">
2708+
<create>
2709+
<node id='-25355' lat='68.13898255618' lon='-105.8206640625' changeset="3" />
2710+
</create>
2711+
</osmChange>)" );
2712+
2713+
// execute the request
2714+
process_request(req, limiter, generator, route, *sel_factory, upd_factory.get());
2715+
2716+
CAPTURE(req.body().str());
2717+
REQUIRE(req.response_status() == 200);
2718+
}
2719+
2720+
{
2721+
test_request req;
2722+
req.set_header("REQUEST_METHOD", "POST");
2723+
req.set_header("REQUEST_URI", "/api/0.6/changeset/3/upload");
2724+
req.set_header("HTTP_AUTHORIZATION", bearertoken);
2725+
req.set_header("REMOTE_ADDR", "127.0.0.1");
2726+
2727+
req.set_payload(R"(<?xml version="1.0" encoding="UTF-8"?>
2728+
<osmChange version="0.6" generator="iD">
2729+
<create>
2730+
<node id='-25357' lat='-34.30685345531' lon='80.8590234375' changeset="3" />
2731+
</create>
2732+
</osmChange>)" );
2733+
2734+
// execute the request
2735+
process_request(req, limiter, generator, route, *sel_factory, upd_factory.get());
2736+
2737+
CAPTURE(req.body().str());
2738+
REQUIRE(req.response_status() == 413);
2739+
}
2740+
}
2741+
2742+
SECTION("Try to upload one way with two nodes, with very small bbox")
2743+
{
2744+
// set up request headers from test case
2745+
test_request req;
2746+
req.set_header("REQUEST_METHOD", "POST");
2747+
req.set_header("REQUEST_URI", "/api/0.6/changeset/1/upload");
2748+
req.set_header("HTTP_AUTHORIZATION", bearertoken);
2749+
req.set_header("REMOTE_ADDR", "127.0.0.1");
2750+
2751+
req.set_payload(R"(<?xml version="1.0" encoding="UTF-8"?>
2752+
<osmChange version="0.6" generator="iD">
2753+
<create>
2754+
<node id='-25360' lat='51.50723246769' lon='-0.12171328202' changeset="1" />
2755+
<node id='-25361' lat='51.50719824397' lon='-0.12160197034' changeset="1" />
2756+
<way id='-582' changeset="1">
2757+
<nd ref='-25360' />
2758+
<nd ref='-25361' />
2759+
</way>
2760+
</create>
2761+
</osmChange>)" );
2762+
2763+
// execute the request
2764+
process_request(req, limiter, generator, route, *sel_factory, upd_factory.get());
2765+
2766+
CAPTURE(req.body().str());
2767+
REQUIRE(req.response_status() == 200);
2768+
2769+
auto doc = getDocument(req.body().str());
2770+
REQUIRE(getXPath(doc.get(), "/diffResult/node[1]/@old_id") == "-25360");
2771+
REQUIRE(getXPath(doc.get(), "/diffResult/node[2]/@old_id") == "-25361");
2772+
REQUIRE(getXPath(doc.get(), "/diffResult/way[1]/@old_id") == "-582");
2773+
REQUIRE(getXPath(doc.get(), "/diffResult/node[1]/@new_version") == "1");
2774+
REQUIRE(getXPath(doc.get(), "/diffResult/node[2]/@new_version") == "1");
2775+
REQUIRE(getXPath(doc.get(), "/diffResult/way[1]/@new_version") == "1");
2776+
}
2777+
}
2778+
26022779
int main(int argc, char *argv[]) {
26032780
Catch::Session session;
26042781

0 commit comments

Comments
 (0)