From 21a23c9abe9df93fe2556c69febe8b7b6fe8db23 Mon Sep 17 00:00:00 2001 From: moson Date: Sun, 25 Feb 2024 10:46:47 +0100 Subject: [PATCH] feat: Limit comment length Limit the amount of characters that can be entered for a comment. Signed-off-by: moson --- aurweb/pkgbase/validate.py | 27 +++++++- aurweb/routers/pkgbase.py | 18 +++-- aurweb/templates.py | 2 + conf/config.defaults | 2 + po/aurweb.pot | 4 ++ templates/partials/packages/comment_form.html | 3 + templates/pkgbase/comments/edit.html | 6 +- templates/pkgbase/delete.html | 2 +- templates/pkgbase/disown.html | 1 + templates/pkgbase/flag.html | 3 +- templates/pkgbase/merge.html | 1 + templates/pkgbase/request.html | 5 +- templates/requests/close.html | 3 +- test/test_pkgbase_routes.py | 65 ++++++++++++++++++- 14 files changed, 128 insertions(+), 14 deletions(-) diff --git a/aurweb/pkgbase/validate.py b/aurweb/pkgbase/validate.py index 3c50e578..b76e1a38 100644 --- a/aurweb/pkgbase/validate.py +++ b/aurweb/pkgbase/validate.py @@ -1,6 +1,9 @@ +from http import HTTPStatus from typing import Any -from aurweb import db +from fastapi import HTTPException + +from aurweb import config, db from aurweb.exceptions import ValidationError from aurweb.models import PackageBase @@ -12,8 +15,8 @@ def request( merge_into: str, context: dict[str, Any], ) -> None: - if not comments: - raise ValidationError(["The comment field must not be empty."]) + # validate comment + comment(comments) if type == "merge": # Perform merge-related checks. @@ -32,3 +35,21 @@ def request( if target.ID == pkgbase.ID: # TODO: This error needs to be translated. raise ValidationError(["You cannot merge a package base into itself."]) + + +def comment(comment: str): + if not comment: + raise ValidationError(["The comment field must not be empty."]) + + if len(comment) > config.getint("options", "max_chars_comment", 5000): + raise ValidationError(["Maximum number of characters for comment exceeded."]) + + +def comment_raise_http_ex(comments: str): + try: + comment(comments) + except ValidationError as err: + raise HTTPException( + status_code=HTTPStatus.BAD_REQUEST, + detail=err.data[0], + ) diff --git a/aurweb/routers/pkgbase.py b/aurweb/routers/pkgbase.py index a91b2b48..213348cd 100644 --- a/aurweb/routers/pkgbase.py +++ b/aurweb/routers/pkgbase.py @@ -159,6 +159,8 @@ async def pkgbase_flag_post( request, "pkgbase/flag.html", context, status_code=HTTPStatus.BAD_REQUEST ) + validate.comment_raise_http_ex(comments) + has_cred = request.user.has_credential(creds.PKGBASE_FLAG) if has_cred and not pkgbase.OutOfDateTS: now = time.utcnow() @@ -185,8 +187,7 @@ async def pkgbase_comments_post( """Add a new comment via POST request.""" pkgbase = get_pkg_or_base(name, PackageBase) - if not comment: - raise HTTPException(status_code=HTTPStatus.BAD_REQUEST) + validate.comment_raise_http_ex(comment) # If the provided comment is different than the record's version, # update the db record. @@ -304,9 +305,9 @@ async def pkgbase_comment_post( pkgbase = get_pkg_or_base(name, PackageBase) db_comment = get_pkgbase_comment(pkgbase, id) - if not comment: - raise HTTPException(status_code=HTTPStatus.BAD_REQUEST) - elif request.user.ID != db_comment.UsersID: + validate.comment_raise_http_ex(comment) + + if request.user.ID != db_comment.UsersID: raise HTTPException(status_code=HTTPStatus.UNAUTHORIZED) # If the provided comment is different than the record's version, @@ -602,6 +603,9 @@ async def pkgbase_disown_post( ): pkgbase = get_pkg_or_base(name, PackageBase) + if comments: + validate.comment_raise_http_ex(comments) + comaints = {c.User for c in pkgbase.comaintainers} approved = [pkgbase.Maintainer] + list(comaints) has_cred = request.user.has_credential(creds.PKGBASE_DISOWN, approved=approved) @@ -873,6 +877,7 @@ async def pkgbase_delete_post( ) if comments: + validate.comment_raise_http_ex(comments) # Update any existing deletion requests' ClosureComment. with db.begin(): requests = pkgbase.requests.filter( @@ -966,6 +971,9 @@ async def pkgbase_merge_post( request, "pkgbase/merge.html", context, status_code=HTTPStatus.BAD_REQUEST ) + if comments: + validate.comment_raise_http_ex(comments) + with db.begin(): update_closure_comment(pkgbase, MERGE_ID, comments, target=target) diff --git a/aurweb/templates.py b/aurweb/templates.py index 9e18014c..51b9d342 100644 --- a/aurweb/templates.py +++ b/aurweb/templates.py @@ -70,6 +70,7 @@ def make_context(request: Request, title: str, next: str = None): commit_url = aurweb.config.get_with_fallback("devel", "commit_url", None) commit_hash = aurweb.config.get_with_fallback("devel", "commit_hash", None) + max_chars_comment = aurweb.config.getint("options", "max_chars_comment", 5000) if commit_hash: # Shorten commit_hash to a short Git hash. commit_hash = commit_hash[:7] @@ -92,6 +93,7 @@ def make_context(request: Request, title: str, next: str = None): "creds": aurweb.auth.creds, "next": next if next else request.url.path, "version": os.environ.get("COMMIT_HASH", aurweb.config.AURWEB_VERSION), + "max_chars_comment": max_chars_comment, } diff --git a/conf/config.defaults b/conf/config.defaults index db885b65..9b3023d7 100644 --- a/conf/config.defaults +++ b/conf/config.defaults @@ -49,6 +49,8 @@ salt_rounds = 12 redis_address = redis://localhost ; Toggles traceback display in templates/errors/500.html. traceback = 0 +; Maximum number of characters for a comment +max_chars_comment = 5000 [ratelimit] request_limit = 4000 diff --git a/po/aurweb.pot b/po/aurweb.pot index 88f9de5d..b1a467e4 100644 --- a/po/aurweb.pot +++ b/po/aurweb.pot @@ -2371,3 +2371,7 @@ msgid "Note that if you hide your email address, it'll " "receive an email. However, replies are typically sent to the " "mailing-list and would then be visible in the archive." msgstr "" + +#: templates/partials/packages/comment_form.html +msgid "Maximum number of characters" +msgstr "" diff --git a/templates/partials/packages/comment_form.html b/templates/partials/packages/comment_form.html index bf9a14d0..cecc496e 100644 --- a/templates/partials/packages/comment_form.html +++ b/templates/partials/packages/comment_form.html @@ -21,12 +21,15 @@ Routes: | format('', "") | safe }} +
+ {{ "Maximum number of characters" | tr }}: {{ max_chars_comment }}.

diff --git a/templates/pkgbase/comments/edit.html b/templates/pkgbase/comments/edit.html index f938287e..0d4ec28a 100644 --- a/templates/pkgbase/comments/edit.html +++ b/templates/pkgbase/comments/edit.html @@ -24,13 +24,17 @@ "" ) | safe }} +
+ {{ "Maximum number of characters" | tr }}: {{ max_chars_comment }}.

+ rows="10" + maxlength="{{ max_chars_comment }}" + >{{ comment.Comments }}

diff --git a/templates/pkgbase/delete.html b/templates/pkgbase/delete.html index 5bd74b72..defcf58f 100644 --- a/templates/pkgbase/delete.html +++ b/templates/pkgbase/delete.html @@ -50,7 +50,7 @@

diff --git a/templates/pkgbase/disown.html b/templates/pkgbase/disown.html index 1aedde4f..c01398c8 100644 --- a/templates/pkgbase/disown.html +++ b/templates/pkgbase/disown.html @@ -55,6 +55,7 @@

diff --git a/templates/pkgbase/flag.html b/templates/pkgbase/flag.html index 0335cf18..11d33029 100644 --- a/templates/pkgbase/flag.html +++ b/templates/pkgbase/flag.html @@ -60,7 +60,8 @@ + cols="50" + maxlength="{{ max_chars_comment }}">

diff --git a/templates/pkgbase/merge.html b/templates/pkgbase/merge.html index 981bd649..4b32da87 100644 --- a/templates/pkgbase/merge.html +++ b/templates/pkgbase/merge.html @@ -52,6 +52,7 @@

diff --git a/templates/pkgbase/request.html b/templates/pkgbase/request.html index 3ffa2d2d..413146f0 100644 --- a/templates/pkgbase/request.html +++ b/templates/pkgbase/request.html @@ -64,7 +64,10 @@

+ rows="5" cols="50" + maxlength="{{ max_chars_comment }}" + >{{ comments or '' }} +

diff --git a/templates/requests/close.html b/templates/requests/close.html index df767eae..ee177811 100644 --- a/templates/requests/close.html +++ b/templates/requests/close.html @@ -26,7 +26,8 @@

+ rows="5" cols="50" maxlength="{{ max_chars_comment }}" + >

diff --git a/test/test_pkgbase_routes.py b/test/test_pkgbase_routes.py index a413fe8a..522bb68b 100644 --- a/test/test_pkgbase_routes.py +++ b/test/test_pkgbase_routes.py @@ -6,7 +6,7 @@ import pytest from fastapi.testclient import TestClient from sqlalchemy import and_ -from aurweb import asgi, db, time +from aurweb import asgi, config, db, time from aurweb.models.account_type import USER_ID, AccountType from aurweb.models.dependency_type import DependencyType from aurweb.models.package import Package @@ -25,6 +25,8 @@ from aurweb.testing.email import Email from aurweb.testing.html import get_errors, get_successes, parse_root from aurweb.testing.requests import Request +max_chars_comment = config.getint("options", "max_chars_comment", 5000) + def package_endpoint(package: Package) -> str: return f"/packages/{package.Name}" @@ -572,6 +574,38 @@ def test_pkgbase_comments( assert "form" in data +def test_pkgbase_comment_exceed_character_limit( + client: TestClient, + user: User, + package: Package, + comment: PackageComment, +): + # Post new comment + cookies = {"AURSID": user.login(Request(), "testPassword")} + pkgbasename = package.PackageBase.Name + endpoint = f"/pkgbase/{pkgbasename}/comments" + + with client as request: + request.cookies = cookies + resp = request.post( + endpoint, + data={"comment": "x" * (max_chars_comment + 1)}, + ) + assert resp.status_code == int(HTTPStatus.BAD_REQUEST) + assert "Maximum number of characters for comment exceeded." in resp.text + # Edit existing + cookies = {"AURSID": user.login(Request(), "testPassword")} + with client as request: + request.cookies = cookies + endp = f"/pkgbase/{pkgbasename}/comments/{comment.ID}" + response = request.post( + endp, + data={"comment": "x" * (max_chars_comment + 1)}, + ) + assert response.status_code == HTTPStatus.BAD_REQUEST + assert "Maximum number of characters for comment exceeded." in resp.text + + def test_pkgbase_comment_edit_unauthorized( client: TestClient, user: User, @@ -935,6 +969,28 @@ def test_pkgbase_request_post_no_comment_error( assert error.text.strip() == expected +def test_pkgbase_request_post_comment_exceed_character_limit( + client: TestClient, user: User, package: Package +): + endpoint = f"/pkgbase/{package.PackageBase.Name}/request" + cookies = {"AURSID": user.login(Request(), "testPassword")} + with client as request: + request.cookies = cookies + resp = request.post( + endpoint, + data={ + "type": "deletion", + "comments": "x" * (max_chars_comment + 1), + }, + ) + assert resp.status_code == int(HTTPStatus.OK) + + root = parse_root(resp.text) + error = root.xpath('//ul[@class="errorlist"]/li')[0] + expected = "Maximum number of characters for comment exceeded." + assert error.text.strip() == expected + + def test_pkgbase_request_post_merge_not_found_error( client: TestClient, user: User, package: Package ): @@ -1087,6 +1143,13 @@ def test_pkgbase_flag( assert resp.status_code == int(HTTPStatus.SEE_OTHER) assert pkgbase.Flagger is None + # Try flagging with a comment that exceeds our character limit. + with client as request: + request.cookies = cookies + data = {"comments": "x" * (max_chars_comment + 1)} + resp = request.post(f"/pkgbase/{pkgbase.Name}/flag", data=data) + assert resp.status_code == int(HTTPStatus.BAD_REQUEST) + # Flag it again. with client as request: request.cookies = cookies