Skip to content

Commit 3a8f18a

Browse files
committed
RDBC-1059 Validate topology command responses are topology-shaped
1 parent b4e547a commit 3a8f18a

2 files changed

Lines changed: 83 additions & 8 deletions

File tree

ravendb/serverwide/commands.py

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,32 @@ def create_request(self, node: ServerNode) -> requests.Request:
2828
url += f"&applicationIdentifier=" + str(self.__application_identifier)
2929
if ".fiddler" in node.url.lower():
3030
url += f"&localUrl={Utils.escape(node.url,False,False)}"
31+
self._last_url = url
3132
return requests.Request(method="GET", url=url)
3233

3334
def set_response(self, response: str, from_cache: bool) -> None:
3435
if response is None:
3536
return
3637

37-
# todo: that's pretty bad way to do that, replace with initialization function that take nested object types
38-
self.result: Topology = Utils.initialize_object(json.loads(response), self._result_class, True)
39-
node_list = []
40-
for node in self.result.nodes:
41-
node_list.append(Utils.initialize_object(node, ServerNode, True))
42-
self.result.nodes = node_list
38+
try:
39+
parsed = json.loads(response)
40+
# todo: replace with an initializer that knows nested object types
41+
self.result: Topology = Utils.initialize_object(parsed, self._result_class, True)
42+
if self.result is None or self.result.nodes is None:
43+
self._throw_unexpected_topology_response(response)
44+
node_list = []
45+
for node in self.result.nodes:
46+
node_list.append(Utils.initialize_object(node, ServerNode, True))
47+
self.result.nodes = node_list
48+
except (json.JSONDecodeError, KeyError, TypeError) as e:
49+
self._throw_unexpected_topology_response(response, e)
50+
51+
def _throw_unexpected_topology_response(self, response: str, inner: Optional[Exception] = None) -> None:
52+
message = (
53+
f"Received an unexpected database topology response from '{getattr(self, '_last_url', '')}'. "
54+
f"This may indicate that the URL does not point to a RavenDB server. Response: {response}"
55+
)
56+
raise RuntimeError(message) from inner
4357

4458

4559
class GetClusterTopologyCommand(RavenCommand[ClusterTopologyResponse]):
@@ -51,14 +65,26 @@ def create_request(self, node: ServerNode) -> requests.Request:
5165
url = f"{node.url}/cluster/topology"
5266
if self.__debug_tag is not None:
5367
url += f"?{self.__debug_tag}"
54-
68+
self._last_url = url
5569
return requests.Request("GET", url)
5670

5771
def set_response(self, response: str, from_cache: bool) -> None:
5872
if response is None:
5973
super()._throw_invalid_response()
6074

61-
self.result: ClusterTopologyResponse = ClusterTopologyResponse.from_json(json.loads(response))
75+
try:
76+
self.result: ClusterTopologyResponse = ClusterTopologyResponse.from_json(json.loads(response))
77+
if self.result is None or self.result.topology is None:
78+
self._throw_unexpected_topology_response(response)
79+
except (json.JSONDecodeError, KeyError, TypeError) as e:
80+
self._throw_unexpected_topology_response(response, e)
81+
82+
def _throw_unexpected_topology_response(self, response: str, inner: Optional[Exception] = None) -> None:
83+
message = (
84+
f"Received an unexpected cluster topology response from '{getattr(self, '_last_url', '')}'. "
85+
f"This may indicate that the URL does not point to a RavenDB server. Response: {response}"
86+
)
87+
raise RuntimeError(message) from inner
6288

6389
def is_read_request(self) -> bool:
6490
return True
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
"""
2+
Unit tests for the 7.2.3 topology-command response validation.
3+
4+
When the URL doesn't point at a RavenDB server, the JSON parse may succeed
5+
but the resulting object won't have the expected fields. The client should
6+
raise a clear "may indicate that the URL does not point to a RavenDB server"
7+
error.
8+
"""
9+
10+
import unittest
11+
12+
from ravendb.serverwide.commands import GetClusterTopologyCommand, GetDatabaseTopologyCommand
13+
14+
15+
class TestGetDatabaseTopologyCommandValidation(unittest.TestCase):
16+
def test_response_without_nodes_raises(self):
17+
cmd = GetDatabaseTopologyCommand()
18+
with self.assertRaises(RuntimeError) as ctx:
19+
cmd.set_response('{"NotATopology": true}', from_cache=False)
20+
self.assertIn("does not point to a RavenDB server", str(ctx.exception))
21+
22+
def test_malformed_json_raises_friendly(self):
23+
cmd = GetDatabaseTopologyCommand()
24+
with self.assertRaises(RuntimeError) as ctx:
25+
cmd.set_response("<html>not json</html>", from_cache=False)
26+
self.assertIn("does not point to a RavenDB server", str(ctx.exception))
27+
28+
def test_none_response_is_silent(self):
29+
cmd = GetDatabaseTopologyCommand()
30+
# None means "no response" — matches existing behavior (no raise).
31+
cmd.set_response(None, from_cache=False)
32+
33+
34+
class TestGetClusterTopologyCommandValidation(unittest.TestCase):
35+
def test_response_missing_topology_raises(self):
36+
cmd = GetClusterTopologyCommand()
37+
with self.assertRaises(RuntimeError) as ctx:
38+
cmd.set_response('{"Leader": "A", "NodeTag": "A"}', from_cache=False)
39+
self.assertIn("does not point to a RavenDB server", str(ctx.exception))
40+
41+
def test_malformed_json_raises_friendly(self):
42+
cmd = GetClusterTopologyCommand()
43+
with self.assertRaises(RuntimeError) as ctx:
44+
cmd.set_response("<html>not json</html>", from_cache=False)
45+
self.assertIn("does not point to a RavenDB server", str(ctx.exception))
46+
47+
48+
if __name__ == "__main__":
49+
unittest.main()

0 commit comments

Comments
 (0)