From 99bdf37ec3f10d3d1ed8bd89a7bd8b1bf04d361f Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Mon, 3 Oct 2016 12:31:01 +0200 Subject: [PATCH] Prevent connect a node to himself Fix https://github.com/GNS3/gns3-gui/issues/1553 --- gns3server/compute/base_node.py | 2 + gns3server/compute/builtin/nodes/cloud.py | 3 ++ gns3server/controller/link.py | 3 ++ .../handlers/api/controller/link_handler.py | 14 ++++--- tests/controller/test_link.py | 16 ++++++++ tests/controller/test_udp_link.py | 9 ++++- tests/handlers/api/controller/test_link.py | 39 +++++++++++++++++++ 7 files changed, 79 insertions(+), 7 deletions(-) diff --git a/gns3server/compute/base_node.py b/gns3server/compute/base_node.py index 49b806fa..995143c5 100644 --- a/gns3server/compute/base_node.py +++ b/gns3server/compute/base_node.py @@ -463,6 +463,7 @@ class BaseNode: :param command: command to send """ + print(self._ubridge_hypervisor) if not self._ubridge_hypervisor or not self._ubridge_hypervisor.is_running(): raise NodeError("Cannot send command '{}': uBridge is not running".format(command)) try: @@ -498,6 +499,7 @@ class BaseNode: """ if self._ubridge_hypervisor and self._ubridge_hypervisor.is_running(): + log.info("Stopping uBridge hypervisor {}:{}".format(self._ubridge_hypervisor.host, self._ubridge_hypervisor.port)) yield from self._ubridge_hypervisor.stop() @asyncio.coroutine diff --git a/gns3server/compute/builtin/nodes/cloud.py b/gns3server/compute/builtin/nodes/cloud.py index e332c8dc..48812888 100644 --- a/gns3server/compute/builtin/nodes/cloud.py +++ b/gns3server/compute/builtin/nodes/cloud.py @@ -259,6 +259,9 @@ class Cloud(BaseNode): self._nios[port_number] = nio try: yield from self._add_ubridge_connection(nio, port_number) + except NodeError as e: + del self._nios[port_number] + raise e # Cleanup stuff except UbridgeError as e: try: diff --git a/gns3server/controller/link.py b/gns3server/controller/link.py index e81de665..3a294c78 100644 --- a/gns3server/controller/link.py +++ b/gns3server/controller/link.py @@ -61,6 +61,9 @@ class Link: self._link_type = port.link_type for other_node in self._nodes: + if other_node["node"] == node: + raise aiohttp.web.HTTPConflict(text="Cannot connect to itself") + if node.node_type in ["nat", "cloud"]: if other_node["node"].node_type in ["nat", "cloud"]: raise aiohttp.web.HTTPConflict(text="It's not allowed to connect a {} to a {}".format(other_node["node"].node_type, node.node_type)) diff --git a/gns3server/handlers/api/controller/link_handler.py b/gns3server/handlers/api/controller/link_handler.py index 20fc97cf..7aea027f 100644 --- a/gns3server/handlers/api/controller/link_handler.py +++ b/gns3server/handlers/api/controller/link_handler.py @@ -64,11 +64,15 @@ class LinkHandler: controller = Controller.instance() project = controller.get_project(request.match_info["project_id"]) link = yield from project.add_link() - for node in request.json["nodes"]: - yield from link.add_node(project.get_node(node["node_id"]), - node.get("adapter_number", 0), - node.get("port_number", 0), - label=node.get("label")) + try: + for node in request.json["nodes"]: + yield from link.add_node(project.get_node(node["node_id"]), + node.get("adapter_number", 0), + node.get("port_number", 0), + label=node.get("label")) + except aiohttp.web_exceptions.HTTPException as e: + yield from project.delete_link(link.id) + raise e response.set_status(201) response.json(link) diff --git a/tests/controller/test_link.py b/tests/controller/test_link.py index 0c92fdca..33cdca2b 100644 --- a/tests/controller/test_link.py +++ b/tests/controller/test_link.py @@ -131,6 +131,22 @@ def test_add_node_cloud_to_cloud(async_run, project, compute): async_run(link.add_node(node2, 0, 4)) +def test_add_node_same_node(async_run, project, compute): + """ + Connection to the same node is not allowed + """ + node1 = Node(project, compute, "node1", node_type="qemu") + node1._ports = [EthernetPort("E0", 0, 0, 4), EthernetPort("E1", 0, 0, 5)] + + link = Link(project) + link.create = AsyncioMagicMock() + link._project.controller.notification.emit = MagicMock() + + async_run(link.add_node(node1, 0, 4)) + with pytest.raises(aiohttp.web.HTTPConflict): + async_run(link.add_node(node1, 0, 5)) + + def test_add_node_serial_to_ethernet(async_run, project, compute): """ Serial to ethernet connection is not allowed diff --git a/tests/controller/test_udp_link.py b/tests/controller/test_udp_link.py index ad0f83fe..ea7995da 100644 --- a/tests/controller/test_udp_link.py +++ b/tests/controller/test_udp_link.py @@ -187,10 +187,15 @@ def test_capture(async_run, project): def test_read_pcap_from_source(project, async_run): compute1 = MagicMock() + node_vpcs = Node(project, compute1, "V1", node_type="vpcs") + node_vpcs._ports = [EthernetPort("E0", 0, 0, 4)] + node_iou = Node(project, compute1, "I1", node_type="iou") + node_iou._ports = [EthernetPort("E0", 0, 3, 1)] + link = UDPLink(project) link.create = AsyncioMagicMock() - async_run(link.add_node(compute1, 0, 4)) - async_run(link.add_node(compute1, 3, 1)) + async_run(link.add_node(node_vpcs, 0, 4)) + async_run(link.add_node(node_iou, 3, 1)) capture = async_run(link.start_capture()) assert link._capture_node is not None diff --git a/tests/handlers/api/controller/test_link.py b/tests/handlers/api/controller/test_link.py index ce7dedbe..a7e3194e 100644 --- a/tests/handlers/api/controller/test_link.py +++ b/tests/handlers/api/controller/test_link.py @@ -84,6 +84,45 @@ def test_create_link(http_controller, tmpdir, project, compute, async_run): assert response.json["link_id"] is not None assert len(response.json["nodes"]) == 2 assert response.json["nodes"][0]["label"]["x"] == 42 + assert len(project.links) == 1 + + +def test_create_link_failure(http_controller, tmpdir, project, compute, async_run): + """ + Make sure the link is deleted if we failed to create the link. + + The failure is trigger by connecting the link to himself + """ + response = MagicMock() + response.json = {"console": 2048} + compute.post = AsyncioMagicMock(return_value=response) + + node1 = async_run(project.add_node(compute, "node1", None, node_type="qemu")) + node1._ports = [EthernetPort("E0", 0, 0, 3), EthernetPort("E0", 0, 0, 4)] + + with asyncio_patch("gns3server.controller.udp_link.UDPLink.create") as mock: + response = http_controller.post("/projects/{}/links".format(project.id), { + "nodes": [ + { + "node_id": node1.id, + "adapter_number": 0, + "port_number": 3, + "label": { + "text": "Text", + "x": 42, + "y": 0 + } + }, + { + "node_id": node1.id, + "adapter_number": 0, + "port_number": 4 + } + ] + }, example=True) + #assert mock.called + assert response.status == 409 + assert len(project.links) == 0 def test_update_link(http_controller, tmpdir, project, compute, async_run):