Skip to content

Commit 5aaf32d

Browse files
committed
router: protect replicasets from modification
The function `vshard.router.routeall` is not marked as internal API, but it returns `router.replicasets` table, which is used by the router itself. And also `vshard.router.route` returns a replicaset object that is used internally. If user modifies the returned from `routeall` or `route` table, then router is broken and cannot continue to work properly after that. Now `routeall` returns a new copy of the replicaset array to user on each call. So user can do whatever he wants with this array, without affecting the internal behavior of the router. Also, each of the replicasets that are returned from `routeall` or `route` is a wrapper over the internal replicaset table. This wrapper contains the private field `_replicaset`, the value of which is the internal replicaset table. User should not touch this field. This wrapper only gives user access to the public replicaset methods: `call`, `callrw`, `callro`, `callbro`, `callre`, `callbre`. Closes #502 NO_DOC=<undocumented part>
1 parent acd7606 commit 5aaf32d

17 files changed

+155
-58
lines changed

test/failover/failover.result

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -120,15 +120,15 @@ priority_order()
120120
- - 1
121121
- unknown zone
122122
...
123-
vshard.router.route(1).uuid == rs_uuid[1]
123+
vshard.router.route(1)._replicaset.uuid == rs_uuid[1]
124124
---
125125
- true
126126
...
127-
vshard.router.route(31).uuid == rs_uuid[2]
127+
vshard.router.route(31)._replicaset.uuid == rs_uuid[2]
128128
---
129129
- true
130130
...
131-
vshard.router.route(61).uuid == rs_uuid[3]
131+
vshard.router.route(61)._replicaset.uuid == rs_uuid[3]
132132
---
133133
- true
134134
...
@@ -371,15 +371,15 @@ priority_order()
371371
- - 1
372372
- unknown zone
373373
...
374-
vshard.router.route(1).uuid == rs_uuid[1]
374+
vshard.router.route(1)._replicaset.uuid == rs_uuid[1]
375375
---
376376
- true
377377
...
378-
vshard.router.route(31).uuid == rs_uuid[2]
378+
vshard.router.route(31)._replicaset.uuid == rs_uuid[2]
379379
---
380380
- true
381381
...
382-
vshard.router.route(61).uuid == rs_uuid[3]
382+
vshard.router.route(61)._replicaset.uuid == rs_uuid[3]
383383
---
384384
- true
385385
...
@@ -409,15 +409,15 @@ priority_order()
409409
- - 1
410410
- unknown zone
411411
...
412-
vshard.router.route(1).uuid == rs_uuid[1]
412+
vshard.router.route(1)._replicaset.uuid == rs_uuid[1]
413413
---
414414
- true
415415
...
416-
vshard.router.route(31).uuid == rs_uuid[2]
416+
vshard.router.route(31)._replicaset.uuid == rs_uuid[2]
417417
---
418418
- true
419419
...
420-
vshard.router.route(61).uuid == rs_uuid[3]
420+
vshard.router.route(61)._replicaset.uuid == rs_uuid[3]
421421
---
422422
- true
423423
...
@@ -447,15 +447,15 @@ priority_order()
447447
- - unknown zone
448448
- 1
449449
...
450-
vshard.router.route(1).uuid == rs_uuid[1]
450+
vshard.router.route(1)._replicaset.uuid == rs_uuid[1]
451451
---
452452
- true
453453
...
454-
vshard.router.route(31).uuid == rs_uuid[2]
454+
vshard.router.route(31)._replicaset.uuid == rs_uuid[2]
455455
---
456456
- true
457457
...
458-
vshard.router.route(61).uuid == rs_uuid[3]
458+
vshard.router.route(61)._replicaset.uuid == rs_uuid[3]
459459
---
460460
- true
461461
...
@@ -482,7 +482,7 @@ while not test_run:grep_log('router_1', 'Ping error from', 1000) do fiber.sleep(
482482
t = string.rep('a', 1024 * 1024 * 500)
483483
---
484484
...
485-
rs = vshard.router.route(31)
485+
rs = vshard.router.route(31)._replicaset
486486
---
487487
...
488488
while rs.master ~= rs.replica do fiber.sleep(0.01) end

test/failover/failover.test.lua

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ test_run:switch('router_1')
5555
vshard.router.cfg(cfg)
5656
while not test_run:grep_log('router_1', 'New replica box_1_d%(storage%@') do fiber.sleep(0.1) end
5757
priority_order()
58-
vshard.router.route(1).uuid == rs_uuid[1]
59-
vshard.router.route(31).uuid == rs_uuid[2]
60-
vshard.router.route(61).uuid == rs_uuid[3]
58+
vshard.router.route(1)._replicaset.uuid == rs_uuid[1]
59+
vshard.router.route(31)._replicaset.uuid == rs_uuid[2]
60+
vshard.router.route(61)._replicaset.uuid == rs_uuid[3]
6161
vshard.router.call(1, 'read', 'echo', {123})
6262
test_run:switch('box_1_d')
6363
-- Not 0 - 'read' echo was called here.
@@ -145,27 +145,27 @@ create_router('router_2')
145145
test_run:switch('router_2')
146146
vshard.router.cfg(cfg)
147147
priority_order()
148-
vshard.router.route(1).uuid == rs_uuid[1]
149-
vshard.router.route(31).uuid == rs_uuid[2]
150-
vshard.router.route(61).uuid == rs_uuid[3]
148+
vshard.router.route(1)._replicaset.uuid == rs_uuid[1]
149+
vshard.router.route(31)._replicaset.uuid == rs_uuid[2]
150+
vshard.router.route(61)._replicaset.uuid == rs_uuid[3]
151151
test_run:switch('default')
152152

153153
create_router('router_3')
154154
test_run:switch('router_3')
155155
vshard.router.cfg(cfg)
156156
priority_order()
157-
vshard.router.route(1).uuid == rs_uuid[1]
158-
vshard.router.route(31).uuid == rs_uuid[2]
159-
vshard.router.route(61).uuid == rs_uuid[3]
157+
vshard.router.route(1)._replicaset.uuid == rs_uuid[1]
158+
vshard.router.route(31)._replicaset.uuid == rs_uuid[2]
159+
vshard.router.route(61)._replicaset.uuid == rs_uuid[3]
160160
test_run:switch('default')
161161

162162
create_router('router_4')
163163
test_run:switch('router_4')
164164
vshard.router.cfg(cfg)
165165
priority_order()
166-
vshard.router.route(1).uuid == rs_uuid[1]
167-
vshard.router.route(31).uuid == rs_uuid[2]
168-
vshard.router.route(61).uuid == rs_uuid[3]
166+
vshard.router.route(1)._replicaset.uuid == rs_uuid[1]
167+
vshard.router.route(31)._replicaset.uuid == rs_uuid[2]
168+
vshard.router.route(61)._replicaset.uuid == rs_uuid[3]
169169

170170
--
171171
-- gh-169: do not close connections on too long ping when this is
@@ -178,7 +178,7 @@ vshard.router.cfg(cfg)
178178
while not test_run:grep_log('router_1', 'Ping error from', 1000) do fiber.sleep(0.01) end
179179

180180
t = string.rep('a', 1024 * 1024 * 500)
181-
rs = vshard.router.route(31)
181+
rs = vshard.router.route(31)._replicaset
182182
while rs.master ~= rs.replica do fiber.sleep(0.01) end
183183
future = nil
184184
-- Create strong reference to prevent Lua GC from closing the

test/misc/check_uuid_on_connect.result

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ vshard.router.route(1)
199199
bucket_id: 1
200200
...
201201
-- Ok to work with correct replicasets.
202-
vshard.router.route(2).uuid
202+
vshard.router.route(2)._replicaset.uuid
203203
---
204204
- cbf06940-0790-498b-948d-042b62cf3d29
205205
...

test/misc/check_uuid_on_connect.test.lua

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ test_run:switch('bad_uuid_router')
7474
vshard.router.static.route_map[1] = nil
7575
vshard.router.route(1)
7676
-- Ok to work with correct replicasets.
77-
vshard.router.route(2).uuid
77+
vshard.router.route(2)._replicaset.uuid
7878

7979
_ = test_run:cmd("switch default")
8080
test_run:cmd('stop server bad_uuid_router')

test/multiple_routers/multiple_routers.result

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,6 @@ vshard.router.call(1, 'read', 'do_select', {1})
7979
---
8080
- [[1, 1]]
8181
...
82-
-- Test that static router is just a router object under the hood.
83-
vshard.router.static:route(1) == vshard.router.route(1)
84-
---
85-
- true
86-
...
8782
-- Configure extra router.
8883
router_2 = vshard.router.new('router_2', configs.cfg_2)
8984
---

test/multiple_routers/multiple_routers.test.lua

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,6 @@ _ = test_run:cmd("switch router_1")
3030
vshard.router.call(1, 'write', 'do_replace', {{1, 1}})
3131
vshard.router.call(1, 'read', 'do_select', {1})
3232

33-
-- Test that static router is just a router object under the hood.
34-
vshard.router.static:route(1) == vshard.router.route(1)
35-
3633
-- Configure extra router.
3734
router_2 = vshard.router.new('router_2', configs.cfg_2)
3835
router_2:bootstrap()

test/router-luatest/map_callrw_test.lua

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ g.test_map_part_double_ref = function(cg)
212212
-- Make sure the location of the bucket is known.
213213
local rs, err = ivshard.router.route(bid)
214214
ilt.assert_equals(err, nil)
215+
rs = rs._replicaset
215216
ilt.assert_equals(rs.uuid, uuid)
216217
end, {bid1, cg.rs1_uuid})
217218
-- Then, move the bucket form rs1 to rs2. Now the router has an outdated
@@ -251,7 +252,7 @@ g.test_map_part_double_ref = function(cg)
251252
ivshard.router.discovery_wakeup()
252253
local rs, err = ivshard.router.route(bid)
253254
ilt.assert_equals(err, nil)
254-
ilt.assert_equals(rs.uuid, uuid)
255+
ilt.assert_equals(rs._replicaset.uuid, uuid)
255256
end)
256257
end, {bid1, cg.rs1_uuid})
257258
end
@@ -273,7 +274,7 @@ g.test_map_part_ref_timeout = function(cg)
273274
for _, bid in ipairs(bids) do
274275
local rs, err = ivshard.router.route(bid)
275276
ilt.assert_equals(err, nil)
276-
ilt.assert_equals(rs.uuid, uuid)
277+
ilt.assert_equals(rs._replicaset.uuid, uuid)
277278
end
278279
end
279280
end, {{[cg.rs1_uuid] = {bid1, bid2}, [cg.rs2_uuid] = {bid3, bid4}}})
@@ -379,7 +380,7 @@ g.test_map_part_ref_timeout = function(cg)
379380
ivshard.router.discovery_wakeup()
380381
local rs, err = ivshard.router.route(bid)
381382
ilt.assert_equals(err, nil)
382-
ilt.assert_equals(rs.uuid, uuid)
383+
ilt.assert_equals(rs._replicaset.uuid, uuid)
383384
end)
384385
end, {bid2, cg.rs1_uuid})
385386
end

test/router-luatest/router_2_2_test.lua

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,65 @@ g.test_msgpack_args = function(g)
122122
t.assert_equals(res, 100, 'good result')
123123
end
124124

125+
local function router_check_replicaset_protection_init()
126+
g.router:exec(function()
127+
rawset(_G, "check_replicaset_protection",
128+
function(rs)
129+
t.assert_not_equals(rs._replicaset, nil)
130+
t.assert_equals(next(rs, next(rs)), nil)
131+
t.assert_not_equals(rs.call, nil)
132+
t.assert_not_equals(rs.callrw, nil)
133+
t.assert_not_equals(rs.callro, nil)
134+
t.assert_not_equals(rs.callbro, nil)
135+
t.assert_not_equals(rs.callre, nil)
136+
t.assert_not_equals(rs.callbre, nil)
137+
local err_msg =
138+
"Modification of replicaset table is not allowed"
139+
-- Adding a field is prohibited.
140+
t.assert_error_msg_contains(
141+
err_msg, function(t) t.new_field = 1 end, rs)
142+
err_msg = "cannot change a protected metatable"
143+
t.assert_error_msg_equals(err_msg, setmetatable, rs, {})
144+
end)
145+
end)
146+
end
147+
148+
local function router_check_replicaset_protection_clear()
149+
g.router:exec(function()
150+
rawset(_G, "check_replicaset_protection", nil)
151+
end)
152+
end
153+
154+
g.test_router_route_return_value_protection = function(g)
155+
router_check_replicaset_protection_init()
156+
g.router:exec(function()
157+
_G.check_replicaset_protection(ivshard.router.route(1))
158+
end)
159+
router_check_replicaset_protection_clear()
160+
end
161+
162+
g.test_router_routeall_return_value_protection = function(g)
163+
router_check_replicaset_protection_init()
164+
g.router:exec(function()
165+
local replicasets = ivshard.router.routeall()
166+
for _, rs in pairs(replicasets) do
167+
_G.check_replicaset_protection(rs)
168+
end
169+
-- Adding a field.
170+
replicasets.new_field = 1
171+
t.assert_equals(replicasets.new_field, 1)
172+
replicasets = ivshard.router.routeall()
173+
t.assert_equals(replicasets.new_field, nil)
174+
-- Deleting a field.
175+
local id, _ = next(replicasets)
176+
replicasets[id] = nil
177+
t.assert_equals(replicasets[id], nil)
178+
replicasets = ivshard.router.routeall()
179+
t.assert_not_equals(replicasets[id], nil)
180+
end)
181+
router_check_replicaset_protection_clear()
182+
end
183+
125184
local function test_return_raw_template(g, mode)
126185
--
127186
-- Normal call.
@@ -1083,7 +1142,7 @@ g.test_retryable_transfer_in_progress = function(g)
10831142
end)
10841143
g.router:exec(function(bid, uuid)
10851144
local rs = ivshard.router.route(bid)
1086-
ilt.assert_equals(rs.uuid, uuid)
1145+
ilt.assert_equals(rs._replicaset.uuid, uuid)
10871146
end, {bid, g.replica_2_a:replicaset_uuid()})
10881147

10891148
-- Block bucket receive.

test/router-luatest/router_election_auto_master_test.lua

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ end)
6767
local function router_wait_for_leader(bid, uuid)
6868
ilt.helpers.retrying({timeout = ivtest.wait_timeout}, function()
6969
ivshard.router.master_search_wakeup()
70-
ilt.assert_equals(ivshard.router.route(bid).master.uuid, uuid)
70+
ilt.assert_equals(
71+
ivshard.router.route(bid)._replicaset.master.uuid, uuid)
7172
end)
7273
end
7374

test/router/reload.result

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ check_reloaded()
203203
--
204204
-- Outdate old replicaset and replica objects.
205205
--
206-
rs = vshard.router.route(1)
206+
rs = vshard.router.route(1)._replicaset
207207
---
208208
...
209209
rs:callro('echo', {'some_data'})
@@ -233,7 +233,7 @@ rs.callro(rs, 'echo', {'some_data'})
233233
vshard.router = require('vshard.router')
234234
---
235235
...
236-
rs = vshard.router.route(1)
236+
rs = vshard.router.route(1)._replicaset
237237
---
238238
...
239239
rs:callro('echo', {'some_data'})
@@ -258,7 +258,7 @@ cfg.connection_outdate_delay = old_connection_delay
258258
vshard.router.static.connection_outdate_delay = nil
259259
---
260260
...
261-
rs_new = vshard.router.route(1)
261+
rs_new = vshard.router.route(1)._replicaset
262262
---
263263
...
264264
rs_old = rs

0 commit comments

Comments
 (0)