Skip to content

Commit

Permalink
Disable SQL server functionality (dask-contrib#448)
Browse files Browse the repository at this point in the history
* Disable SQL server functionality

* Update docs/source/server.rst

Co-authored-by: Ayush Dattagupta <[email protected]>

* Disable server at lowest possible level

* Skip all server tests

* Add tests to ensure server is disabled

* Fix CVE fix test

Co-authored-by: Ayush Dattagupta <[email protected]>
  • Loading branch information
charlesbluca and ayushdg authored Apr 7, 2022
1 parent 50d95d2 commit 37a3a61
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 35 deletions.
6 changes: 4 additions & 2 deletions continuous_integration/recipe/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ build:
number: {{ GIT_DESCRIBE_NUMBER }}
noarch: python
entry_points:
- dask-sql-server = dask_sql.server.app:main
# TODO: re-enable server once CVEs are resolved
# - dask-sql-server = dask_sql.server.app:main
- dask-sql = dask_sql.cmd:main
string: py_{{ GIT_DESCRIBE_HASH }}_{{ GIT_DESCRIBE_NUMBER }}
script: {{ PYTHON }} -m pip install . --no-deps -vv
Expand Down Expand Up @@ -45,7 +46,8 @@ test:
- dask_sql
commands:
- pip check
- dask-sql-server --help
# TODO: re-enable server once CVEs are resolved
# - dask-sql-server --help
- dask-sql --help
requires:
- pip
Expand Down
7 changes: 5 additions & 2 deletions dask_sql/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
from .cmd import cmd_loop
from .context import Context
from .datacontainer import Statistics
from .server.app import run_server

# from .server.app import run_server

__version__ = get_versions()["version"]
del get_versions

__all__ = [__version__, cmd_loop, Context, run_server, Statistics]
# TODO: re-enable server once CVEs are resolved
# __all__ = [__version__, cmd_loop, Context, run_server, Statistics]
__all__ = [__version__, cmd_loop, Context, Statistics]
2 changes: 1 addition & 1 deletion dask_sql/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ def run_server(
from dask_sql.server.app import run_server

self.stop_server()
self.server = run_server(
self.sql_server = run_server(
context=self,
client=client,
host=host,
Expand Down
3 changes: 3 additions & 0 deletions dask_sql/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,9 @@ def _init_app(
context: Context = None,
client: dask.distributed.Client = None,
):
# TODO: re-enable server once CVEs are resolved
raise NotImplementedError

app.c = context or Context()
app.future_list = {}

Expand Down
4 changes: 4 additions & 0 deletions docs/source/server.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
SQL Server
==========

.. warning::

``dask-sql``'s SQL server functionality is currently exploitable and has been disabled until the exposed vulnerabilities can be resolved.

``dask-sql`` comes with a small test implementation for a SQL server.
Instead of rebuilding a full ODBC driver, we re-use the `presto wire protocol <https://github.com/prestodb/presto/wiki/HTTP-Protocol>`_.

Expand Down
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ def build(self):
},
entry_points={
"console_scripts": [
"dask-sql-server = dask_sql.server.app:main",
# TODO: re-enable server once CVEs are resolved
# "dask-sql-server = dask_sql.server.app:main",
"dask-sql = dask_sql.cmd:main",
]
},
Expand Down
16 changes: 16 additions & 0 deletions tests/integration/test_cve_fix.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import pytest

from dask_sql import Context
from dask_sql.server.app import _init_app, app


def test_run_server_disabled(c):
with pytest.raises(NotImplementedError):
c.run_server()


def test_init_app_disabled():
c = Context()
c.sql("SELECT 1 + 1").compute()
with pytest.raises(NotImplementedError):
_init_app(app, c)
5 changes: 5 additions & 0 deletions tests/integration/test_jdbc.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
from dask_sql.server.app import _init_app, app
from dask_sql.server.presto_jdbc import create_meta_data

# TODO: re-enable server once CVEs are resolved
pytest.skip(
"SQL server is disabled until related CVEs are resolved", allow_module_level=True
)

# needed for the testclient
pytest.importorskip("requests")

Expand Down
63 changes: 34 additions & 29 deletions tests/integration/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
from dask_sql import Context
from dask_sql.server.app import _init_app, app

# TODO: re-enable server once CVEs are resolved
pytest.skip(
"SQL server is disabled until related CVEs are resolved", allow_module_level=True
)

# needed for the testclient
pytest.importorskip("requests")

Expand All @@ -23,6 +28,35 @@ def app_client():
app.client.close()


def get_result_or_error(app_client, response):
result = response.json()

assert "nextUri" in result
assert "error" not in result

status_url = result["nextUri"]
next_url = status_url

counter = 0
while True:
response = app_client.get(next_url)
assert response.status_code == 200

result = response.json()

if "nextUri" not in result:
break

next_url = result["nextUri"]

counter += 1
assert counter <= 100

sleep(0.1)

return result


def test_routes(app_client):
assert app_client.post("/v1/statement", data="SELECT 1 + 1").status_code == 200
assert app_client.get("/v1/statement", data="SELECT 1 + 1").status_code == 405
Expand Down Expand Up @@ -174,32 +208,3 @@ def test_inf_table(app_client, user_table_inf):
assert len(result["data"]) == 3
assert result["data"][1] == ["+Infinity"]
assert "error" not in result


def get_result_or_error(app_client, response):
result = response.json()

assert "nextUri" in result
assert "error" not in result

status_url = result["nextUri"]
next_url = status_url

counter = 0
while True:
response = app_client.get(next_url)
assert response.status_code == 200

result = response.json()

if "nextUri" not in result:
break

next_url = result["nextUri"]

counter += 1
assert counter <= 100

sleep(0.1)

return result

0 comments on commit 37a3a61

Please sign in to comment.