Add some more checks and fix them, also don't leak password (fixes #104…

… and #93) (#105)

* pass default pylama checks

* fix formatting error

* just use flake8

* fix spacing

* fix more warnings

* fix issue #93

* make mypy happy

* fix doc build by slight restructure

* prepare for 1.6.0

* doc shuffle

* Document handle_download

* remove one more empty line

Co-authored-by: Joeri van Ruth <joeri.van.ruth@monetdbsolutions.com>

CHANGES

@@ -1,3 +1,13 @@
+# 1.6.0
+
+changes since 1.5.1
+
+* Support for COPY INTO ON CLIENT (#57)
+* Connection object leaks the password (#93)
+* Make code style checking stricter (mypy and flake8) (#104)
+* Make python UDF debug code work with Python3 (#45 )
+
+
 # 1.5.1
 
 changes since 1.5.0

Makefile

@@ -3,9 +3,11 @@
 DBFARM=dbfarm
 DATABASE=demo
 
-.PHONY: doc clean test wheel sdist dbfarm-start database-init
+.PHONY: doc clean test wheel sdist dbfarm-start database-init all build
 
-all: test
+all: test doc checks build
+
+build: wheel sdist
 
 venv/:
 	python3 -m venv venv
@@ -51,15 +53,22 @@ upload: venv/bin/twine wheel sdist
 	venv/bin/twine upload dist/*.whl dist/*.tar.gz
 
 doc: setup
-	PATH=$${PATH}:${CURDIR}/venv/bin $(MAKE) -C doc html
+	 PATH=$${PATH}:${CURDIR}/venv/bin $(MAKE) -C doc html SPHINXOPTS="-W"
 
 venv/bin/flake8: setup
 	venv/bin/pip install flake8
 	touch venv/bin/flake8
 
 flake8: venv/bin/flake8
 	venv/bin/flake8 --count --select=E9,F63,F7,F82 --show-source --statistics pymonetdb tests
-	venv/bin/flake8 --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics pymonetdb tests
+	venv/bin/flake8 --count --max-complexity=10 --max-line-length=127 --statistics pymonetdb tests
+
+venv/bin/pylama: setup
+	venv/bin/pip install "pylama[all]"
+	touch venv/bin/pylama
+
+pylama: venv/bin/pylama
+	venv/bin/pylama pymonetdb tests
 
 checks: mypy pycodestyle flake8
 

doc/api.rst

@@ -44,15 +44,9 @@ File Uploads and Downloads
 Classes related to file transfer requests as used by COPY INTO ON CLIENT.
 
 .. automodule:: pymonetdb.filetransfer
-    :members: Uploader, Downloader, SafeDirectoryHandler
+    :members: Upload, Uploader, Download, Downloader, SafeDirectoryHandler
     :member-order: bysource
 
-.. automodule:: pymonetdb.filetransfer.uploads
-    :members: Upload
-
-.. automodule:: pymonetdb.filetransfer.downloads
-    :members: Download
-
 
 MonetDB remote control
 ======================

pymonetdb/__init__.py

@@ -14,15 +14,15 @@
 from pymonetdb import sql
 from pymonetdb import mapi
 from pymonetdb import exceptions
-from pymonetdb import profiler
 
 from pymonetdb.profiler import ProfilerConnection
 from pymonetdb.sql.connections import Connection
-from pymonetdb.sql.pythonize import *
-from pymonetdb.exceptions import *
-from pymonetdb.filetransfer import Downloader, Uploader
-from pymonetdb.filetransfer.downloads import Download
-from pymonetdb.filetransfer.uploads import Upload
+from pymonetdb.sql.pythonize import BINARY, Binary, DATE, Date, Time, Timestamp, DateFromTicks, TimestampFromTicks, \
+    TimeFromTicks, NUMBER, ROWID, STRING, TIME, types, DATETIME, TimeTzFromTicks, TimestampTzFromTicks
+from pymonetdb.exceptions import Error, DataError, DatabaseError, IntegrityError, InterfaceError, InternalError, \
+    NotSupportedError, OperationalError, ProgrammingError, Warning
+from pymonetdb.filetransfer.downloads import Download, Downloader
+from pymonetdb.filetransfer.uploads import Upload, Uploader
 from pymonetdb.filetransfer.directoryhandler import SafeDirectoryHandler
 
 try:
@@ -38,7 +38,8 @@
            'Timestamp', 'DateFromTicks', 'TimeFromTicks', 'TimestampFromTicks', 'DataError', 'DatabaseError', 'Error',
            'IntegrityError', 'InterfaceError', 'InternalError', 'NUMBER', 'NotSupportedError', 'OperationalError',
            'ProgrammingError', 'ROWID', 'STRING', 'TIME', 'Warning', 'apilevel', 'connect', 'paramstyle',
-           'threadsafety', 'Download', 'Downloader', 'Upload', 'Uploader', 'SafeDirectoryHandler']
+           'threadsafety', 'Download', 'Downloader', 'Upload', 'Uploader', 'SafeDirectoryHandler', 'types', 'DATETIME',
+           'TimeTzFromTicks', 'TimestampTzFromTicks']
 
 
 def connect(*args, **kwargs) -> Connection:

pymonetdb/filetransfer/__init__.py

@@ -3,16 +3,21 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 #
 # Copyright 1997 - July 2008 CWI, August 2008 - 2016 MonetDB B.V.
+import typing
 
-
-from abc import ABC, abstractmethod
-from pymonetdb import mapi as mapi_protocol
 from pymonetdb.exceptions import ProgrammingError
-import pymonetdb.filetransfer.uploads
-import pymonetdb.filetransfer.downloads
+from .uploads import Uploader, Upload
+from .downloads import Downloader, Download
+from .directoryhandler import SafeDirectoryHandler
+
+if typing.TYPE_CHECKING:
+    from pymonetdb.mapi import Connection
+
+# these are used in the code but they are referred to in the docs
+(Uploader, Downloader, SafeDirectoryHandler)
 
 
-def handle_file_transfer(mapi: "mapi_protocol.Connection", cmd: str):
+def handle_file_transfer(mapi: "Connection", cmd: str):
     if cmd.startswith("r "):
         parts = cmd[2:].split(' ', 2)
         if len(parts) == 2:
@@ -33,12 +38,12 @@ def handle_file_transfer(mapi: "mapi_protocol.Connection", cmd: str):
     mapi._putblock(f"Invalid file transfer command: {cmd!r}")
 
 
-def handle_upload(mapi: "mapi_protocol.Connection", filename: str, text_mode: bool, offset: int):
+def handle_upload(mapi: "Connection", filename: str, text_mode: bool, offset: int):
     if not mapi.uploader:
         mapi._putblock("No upload handler has been registered with pymonetdb\n")
         return
     skip_amount = offset - 1 if offset > 0 else 0
-    upload = pymonetdb.filetransfer.uploads.Upload(mapi)
+    upload = Upload(mapi)
     try:
         mapi.uploader.handle_upload(upload, filename, text_mode, skip_amount)
     except Exception as e:
@@ -54,11 +59,11 @@ def handle_upload(mapi: "mapi_protocol.Connection", filename: str, text_mode: bo
         raise ProgrammingError("Upload handler didn't do anything")
 
 
-def handle_download(mapi: "mapi_protocol.Connection", filename: str, text_mode: bool):
+def handle_download(mapi: "Connection", filename: str, text_mode: bool):
     if not mapi.downloader:
         mapi._putblock("No download handler has been registered with pymonetdb\n")
         return
-    download = pymonetdb.filetransfer.downloads.Download(mapi)
+    download = Download(mapi)
     try:
         mapi.downloader.handle_download(download, filename, text_mode)
     except Exception as e:
@@ -83,77 +88,3 @@ def handle_download(mapi: "mapi_protocol.Connection", filename: str, text_mode:
         raise e
     finally:
         download.close()
-
-
-class Uploader(ABC):
-    """
-    Base class for upload hooks. Instances of subclasses of this class can be
-    registered using pymonetdb.Connection.set_uploader(). Every time an upload
-    request is received, an Upload object is created and passed to this objects
-    .handle_upload() method.
-
-    If the server cancels the upload halfway, the .cancel() methods is called
-    and all further data written is ignored.
-    """
-
-    @abstractmethod
-    def handle_upload(self, upload: "pymonetdb.filetransfer.uploads.Upload", filename: str, text_mode: bool, skip_amount: int):
-        """
-        Called when an upload request is received. Implementations should either
-        send an error using upload.send_error(), or request a writer using
-        upload.text_writer() or upload.binary_writer(). All data written to the
-        writer will be sent to the server.
-
-        Parameter 'filename' is the file name used in the COPY INTO statement.
-        Parameter 'text_mode' indicates whether the server requested a text file
-        or a binary file. In case of a text file, 'skip_amount' indicates the
-        number of lines to skip. In binary mode, 'skip_amount' is always 0.
-
-        SECURITY NOTE! Make sure to carefully validate the file name before
-        opening files on the file system. Otherwise, if an adversary has taken
-        control of the network connection or of the server, they can use file
-        upload requests to read arbitrary files from your computer
-        (../../)
-
-        """
-        pass
-
-    def cancel(self):
-        """Optional method called when the server cancels the upload."""
-        pass
-
-
-class Downloader(ABC):
-    """
-    Base class for download hooks. Instances of subclasses of this class can be
-    registered using pymonetdb.Connection.set_downloader(). Every time a
-    download request arrives, a Download object is created and passed to this
-    objects .handle_download() method.
-
-    SECURITY NOTE! Make sure to carefully validate the file name before opening
-    files on the file system. Otherwise, if an adversary has taken control of
-    the network connection or of the server, they can use download requests to
-    OVERWRITE ARBITRARY FILES on your computer
-    """
-
-    @abstractmethod
-    def handle_download(self, download: "pymonetdb.filetransfer.downloads.Download", filename: str, text_mode: bool):
-        """
-        Called when a download request is received from the server. Implementations
-        should either refuse by sending an error using download.send_error(), or
-        request a reader using download.binary_reader() or download.text_reader().
-
-        Parameter 'filename' is the file name used in the COPY INTO statement.
-        Parameter 'text_mode' indicates whether the server requested to send a binary
-        file or a text file.
-
-        SECURITY NOTE! Make sure to carefully validate the file name before opening
-        files on the file system. Otherwise, if an adversary has taken control of
-        the network connection or of the server, they can use download requests to
-        OVERWRITE ARBITRARY FILES on your computer
-        """
-        pass
-
-
-# Only import this at the end to avoid circular imports
-from pymonetdb.filetransfer.directoryhandler import SafeDirectoryHandler

pymonetdb/filetransfer/directoryhandler.py

@@ -13,9 +13,8 @@
 from pathlib import Path
 from shutil import copyfileobj
 from typing import Optional
-from pymonetdb.filetransfer import Uploader, Downloader
-from pymonetdb.filetransfer.uploads import Upload
-from pymonetdb.filetransfer.downloads import Download
+from pymonetdb.filetransfer.uploads import Upload, Uploader
+from pymonetdb.filetransfer.downloads import Download, Downloader
 
 
 class SafeDirectoryHandler(Uploader, Downloader):
@@ -62,7 +61,7 @@ def secure_resolve(self, filename: str) -> Optional[Path]:
         else:
             return None
 
-    def handle_upload(self, upload: Upload, filename: str, text_mode: bool, skip_amount: int):
+    def handle_upload(self, upload: Upload, filename: str, text_mode: bool, skip_amount: int):  # noqa: C901
         """:meta private:"""  # keep the API docs cleaner, this has already been documented on class Uploader.
 
         p = self.secure_resolve(filename)
@@ -162,4 +161,4 @@ def lookup_compression_algorithm(filename: str):
     else:
         return open
     opener = import_module(mod).open
-    return opener
+    return opener

pymonetdb/filetransfer/downloads.py

@@ -129,4 +129,19 @@ class Downloader(ABC):
 
     @abstractmethod
     def handle_download(self, download: Download, filename: str, text_mode: bool):
+        """
+        Called when a download request is received. Implementations should either
+        send an error using download.send_error(), or request a reader using
+        download.text_reader() or download.binary_reader().
+
+        Parameter 'filename' is the file name used in the COPY INTO statement.
+        Parameter 'text_mode' indicates whether the server requested text
+        or binary mode.
+
+        SECURITY NOTE! Make sure to carefully validate the file name before
+        opening files on the file system. Otherwise, if an adversary has taken
+        control of the network connection or of the server, they can use file
+        download requests to overwrite arbitrary files on your computer.
+        (../../)
+        """
         pass

pymonetdb/filetransfer/uploads.py

@@ -6,20 +6,23 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 #
 # Copyright 1997 - July 2008 CWI, August 2008 - 2016 MonetDB B.V.
-
-
+import typing
 from io import BufferedIOBase, BufferedWriter, RawIOBase, TextIOBase, TextIOWrapper
+from abc import ABC, abstractmethod
 from typing import Any, Optional, Union
-from pymonetdb import mapi as mapi_protocol
+from pymonetdb.mapi import MSG_MORE, MSG_FILETRANS
 from pymonetdb.exceptions import ProgrammingError
 
+if typing.TYPE_CHECKING:
+    from pymonetdb.mapi import Connection as MapiConnection
+
 
 class Upload:
     """
     Represents a request from the server to upload data to the server. It is
     passed to the Uploader registered by the application, which for example
     might retrieve the data from a file on the client system. See
-    pymonetdb.Connection.set_uploader().
+    pymonetdb.sql.connections.Connection.set_uploader().
 
     Use the method send_error() to refuse the upload, binary_writer() to get a
     binary file object to write to, or text_writer() to get a text-mode file
@@ -29,7 +32,7 @@ class Upload:
     opening any files on the client system!
     """
 
-    mapi: Optional["mapi_protocol.Connection"]
+    mapi: Optional["MapiConnection"]
     error = False
     cancelled = False
     bytes_sent = 0
@@ -39,7 +42,7 @@ class Upload:
     writer: Optional[BufferedWriter] = None
     twriter: Optional[TextIOBase] = None
 
-    def __init__(self, mapi: "mapi_protocol.Connection"):
+    def __init__(self, mapi: "MapiConnection"):
         self.mapi = mapi
 
     def _check_usable(self):
@@ -141,10 +144,10 @@ def _send_and_get_prompt(self, data: Union[bytes, memoryview]) -> bool:
         assert self.mapi
         self._send(data, True)
         prompt = self.mapi._getblock()
-        if prompt == mapi_protocol.MSG_MORE:
+        if prompt == MSG_MORE:
             self.chunk_used = 0
             return True
-        elif prompt == mapi_protocol.MSG_FILETRANS:
+        elif prompt == MSG_FILETRANS:
             # server says stop
             return False
         else:
@@ -170,7 +173,7 @@ def close(self):
                 self.mapi._putblock('')
                 # receive acknowledgement
                 resp = self.mapi._getblock()
-                if resp != mapi_protocol.MSG_FILETRANS:
+                if resp != MSG_FILETRANS:
                     raise ProgrammingError(f"Unexpected server response: {resp[:50]!r}")
             self.mapi = None
 
@@ -243,3 +246,40 @@ def close(self):
             self.pending = False
         return self.inner.close()
 
+
+class Uploader(ABC):
+    """
+    Base class for upload hooks. Instances of subclasses of this class can be
+    registered using pymonetdb.Connection.set_uploader(). Every time an upload
+    request is received, an Upload object is created and passed to this objects
+    .handle_upload() method.
+
+    If the server cancels the upload halfway, the .cancel() methods is called
+    and all further data written is ignored.
+    """
+
+    @abstractmethod
+    def handle_upload(self, upload: Upload, filename: str, text_mode: bool,
+                      skip_amount: int):
+        """
+        Called when an upload request is received. Implementations should either
+        send an error using upload.send_error(), or request a writer using
+        upload.text_writer() or upload.binary_writer(). All data written to the
+        writer will be sent to the server.
+
+        Parameter 'filename' is the file name used in the COPY INTO statement.
+        Parameter 'text_mode' indicates whether the server requested a text file
+        or a binary file. In case of a text file, 'skip_amount' indicates the
+        number of lines to skip. In binary mode, 'skip_amount' is always 0.
+
+        SECURITY NOTE! Make sure to carefully validate the file name before
+        opening files on the file system. Otherwise, if an adversary has taken
+        control of the network connection or of the server, they can use file
+        upload requests to read arbitrary files from your computer
+        (../../)
+        """
+        pass
+
+    def cancel(self):
+        """Optional method called when the server cancels the upload."""
+        pass