From 7f5021c568b51072f7253c7c3f208565734aaf72 Mon Sep 17 00:00:00 2001
From: dogeystamp <dogeystamp@disroot.org>
Date: Tue, 18 Jul 2023 13:26:48 -0400
Subject: [PATCH] /files: better malformed UUID handling

---
 TODO.txt                     |  3 +++
 sachet/server/files/views.py | 22 +++++++++++++++-------
 tests/test_files.py          | 24 ++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/TODO.txt b/TODO.txt
index 8dbff21..63ca0fc 100644
--- a/TODO.txt
+++ b/TODO.txt
@@ -249,11 +249,14 @@
 	[x] tests
 	[x] docs
 
+[ ] expose filesize in share info
+
 [ ] investigate cleanup being in the user subcmd
 [ ] investigate cleanup cmd triggering foreign key failure
 
 [ ] if you create a new user without a required field it gives 500
 [ ] if you create a new user with the same name as an existing one it gives 500
+[x] if you try to read a share with an invalid uuid it gives 500
 
 [ ] investigate if you can interfere with an upload by setting the same id
 
diff --git a/sachet/server/files/views.py b/sachet/server/files/views.py
index e72f8e0..3d2b052 100644
--- a/sachet/server/files/views.py
+++ b/sachet/server/files/views.py
@@ -9,15 +9,23 @@ from sachet.server import storage, db
 files_blueprint = Blueprint("files_blueprint", __name__)
 
 
+def filter_id(id: str):
+    try:
+        uuid.UUID(id)
+        return id
+    except ValueError:
+        return uuid.UUID(int=0)
+
+
 class FilesMetadataAPI(ModelAPI):
     @auth_required(required_permissions=(Permissions.READ,), allow_anonymous=True)
     def get(self, share_id, auth_user=None):
-        share = Share.query.filter_by(share_id=share_id).first()
+        share = Share.query.filter_by(share_id=filter_id(share_id)).first()
         return super().get(share)
 
     @auth_required(required_permissions=(Permissions.MODIFY,), allow_anonymous=True)
     def patch(self, share_id, auth_user=None):
-        share = Share.query.filter_by(share_id=share_id).first()
+        share = Share.query.filter_by(share_id=filter_id(share_id)).first()
         if not share:
             resp = {
                 "status": "fail",
@@ -53,7 +61,7 @@ class FilesMetadataAPI(ModelAPI):
 
     @auth_required(required_permissions=(Permissions.MODIFY,), allow_anonymous=True)
     def put(self, share_id, auth_user=None):
-        share = Share.query.filter_by(share_id=share_id).first()
+        share = Share.query.filter_by(share_id=filter_id(share_id)).first()
         if not share:
             resp = {
                 "status": "fail",
@@ -88,7 +96,7 @@ class FilesMetadataAPI(ModelAPI):
 
     @auth_required(required_permissions=(Permissions.DELETE,), allow_anonymous=True)
     def delete(self, share_id, auth_user=None):
-        share = Share.query.filter_by(share_id=share_id).first()
+        share = Share.query.filter_by(share_id=filter_id(share_id)).first()
         if not share:
             resp = {
                 "status": "fail",
@@ -178,7 +186,7 @@ class FileContentAPI(MethodView):
 
     @auth_required(required_permissions=(Permissions.CREATE,), allow_anonymous=True)
     def post(self, share_id, auth_user=None):
-        share = Share.query.filter_by(share_id=uuid.UUID(share_id)).first()
+        share = Share.query.filter_by(share_id=filter_id(share_id)).first()
 
         if not share:
             return (
@@ -211,7 +219,7 @@ class FileContentAPI(MethodView):
 
     @auth_required(required_permissions=(Permissions.MODIFY,), allow_anonymous=True)
     def put(self, share_id, auth_user=None):
-        share = Share.query.filter_by(share_id=share_id).first()
+        share = Share.query.filter_by(share_id=filter_id(share_id)).first()
         if not share:
             return (
                 jsonify({"status": "fail", "message": "This share does not exist."})
@@ -248,7 +256,7 @@ class FileContentAPI(MethodView):
 
     @auth_required(required_permissions=(Permissions.READ,), allow_anonymous=True)
     def get(self, share_id, auth_user=None):
-        share = Share.query.filter_by(share_id=share_id).first()
+        share = Share.query.filter_by(share_id=filter_id(share_id)).first()
         if not share:
             return (
                 jsonify({"status": "fail", "message": "This share does not exist."})
diff --git a/tests/test_files.py b/tests/test_files.py
index 82463fe..1ae60e7 100644
--- a/tests/test_files.py
+++ b/tests/test_files.py
@@ -180,6 +180,30 @@ class TestSuite:
         )
         assert resp.status_code == 404
 
+        # malformed
+        resp = client.get("/files/" + "123123123123", headers=auth("jeff"))
+        assert resp.status_code == 404
+        resp = client.get("/files/" + "123123123" + "/content", headers=auth("jeff"))
+        assert resp.status_code == 404
+        resp = client.post("/files/" + "123123123" + "/content", headers=auth("jeff"))
+        assert resp.status_code == 404
+        resp = client.put("/files/" + "123123123" + "/content", headers=auth("jeff"))
+        assert resp.status_code == 404
+        resp = client.delete("/files/" + "123123123", headers=auth("jeff"))
+        assert resp.status_code == 404
+        resp = client.patch(
+            "/files/" + "123123123",
+            headers=auth("jeff"),
+            json=dict(filename="incredible-new-filename"),
+        )
+        assert resp.status_code == 404
+        resp = client.put(
+            "/files/" + "123123123",
+            headers=auth("jeff"),
+            json=dict(filename="incredible-new-filename", owner_name="jeff"),
+        )
+        assert resp.status_code == 404
+
         # no CREATE permission
         resp = client.post("/files", headers=auth("no_create_user"))
         assert resp.status_code == 403