From 65eadaca2c67f861488618810c2e475bb96fefba Mon Sep 17 00:00:00 2001 From: Jerakin Date: Tue, 27 Feb 2024 16:24:00 +0100 Subject: [PATCH] fix: allowed and disallowed file names Also adds tests to make sure it doesn't break again --- src/wikmd/utils.py | 7 ++++- src/wikmd/wiki.py | 34 +++++++++++---------- tests/test_basics.py | 70 ++++++++++++++++++++++++++++++++++++++------ 3 files changed, 86 insertions(+), 25 deletions(-) diff --git a/src/wikmd/utils.py b/src/wikmd/utils.py index 283fae0..62acc56 100644 --- a/src/wikmd/utils.py +++ b/src/wikmd/utils.py @@ -1,6 +1,8 @@ import os import unicodedata +import re +_filename_ascii_strip_re = re.compile(r"[^A-Za-z0-9 _.-]") _windows_device_files = { "CON", "PRN", @@ -21,7 +23,9 @@ def secure_filename(filename: str) -> str: for sep in os.sep, os.path.altsep: if sep: filename = filename.replace(sep, "_") - filename = filename.strip("._") + filename = str(_filename_ascii_strip_re.sub("", filename)).strip( + "._" + ) # on nt a couple of special files are present in each folder. We # have to ensure that the target file is not such a filename. In # this case we prepend an underline @@ -34,6 +38,7 @@ def secure_filename(filename: str) -> str: return filename + def pathify(path1, path2): """ Joins two paths and eventually converts them from Win (\\) to linux OS separator. diff --git a/src/wikmd/wiki.py b/src/wikmd/wiki.py index 8e347c8..e84ba3a 100644 --- a/src/wikmd/wiki.py +++ b/src/wikmd/wiki.py @@ -96,23 +96,27 @@ def process(content: str, page_name: str): def ensure_page_can_be_created(page, page_name): filename = safe_join(cfg.wiki_directory, f"{page_name}.md") - path_exists = os.path.exists(filename) - safe_name = "/".join([secure_filename(part) for part in page_name.split("/")]) - filename_is_ok = safe_name == page_name - if not path_exists and filename_is_ok and page_name: # Early exist - return - - if path_exists: - flash('A page with that name already exists. The page name needs to be unique.') - app.logger.info(f"Page name exists >>> {page_name}.") - - if not filename_is_ok: - flash(f"Page name not accepted. Try using '{safe_name}'.") + if filename is None: + flash(f"Page name not accepted. Contains disallowed characters.") app.logger.info(f"Page name isn't secure >>> {page_name}.") + else: + path_exists = os.path.exists(filename) + safe_name = "/".join([secure_filename(part) for part in page_name.split("/")]) + filename_is_ok = safe_name == page_name + if not path_exists and filename_is_ok and page_name: # Early exist + return + + if path_exists: + flash('A page with that name already exists. The page name needs to be unique.') + app.logger.info(f"Page name exists >>> {page_name}.") + + if not filename_is_ok: + flash(f"Page name not accepted. Try using '{safe_name}'.") + app.logger.info(f"Page name isn't secure >>> {page_name}.") - if not page_name: - flash(f"Your page needs a name.") - app.logger.info(f"No page name provided.") + if not page_name: + flash(f"Your page needs a name.") + app.logger.info(f"No page name provided.") content = process(request.form['CT'], page_name) return render_template("new.html", content=content, title=page, upload_path=cfg.images_route, diff --git a/tests/test_basics.py b/tests/test_basics.py index fccb24e..fd0e6bc 100644 --- a/tests/test_basics.py +++ b/tests/test_basics.py @@ -4,9 +4,20 @@ import pytest from wikmd import wiki -from wikmd.wiki import app, cfg +from wikmd.wiki import app, cfg, setup_wiki_template + + +@pytest.fixture(scope="function", autouse=True) +def wiki_path(tmp_path: Path): + """Sets up the temporary wiki path. + autouse=True is needed as this behaves as a setup for the tests. + """ + wiki_path = tmp_path / "wiki" + wiki_path.mkdir() + cfg.wiki_directory = wiki_path.as_posix() + setup_wiki_template() + return wiki_path -cfg.wiki_directory = (Path(__file__).parent.parent / "src" / "wikmd" / "wiki_template").as_posix() @pytest.fixture() def client(): @@ -19,8 +30,8 @@ def test_file_content(): @pytest.fixture() -def project_file(test_file_content): - testing_folder = Path(cfg.wiki_directory) / "testing_folder" +def project_file(wiki_path, test_file_content): + testing_folder = wiki_path / "testing_folder" testing_folder.mkdir() test_file = testing_folder / "test.md" @@ -56,7 +67,7 @@ def test_list(): def test_create_file_in_folder(wiki_file, test_file_content): - """Make sure the created file is accessible.""" + """Test for accessing file that exists, GET '/{file_name}'.""" rv = app.test_client().get(wiki_file) assert rv.status_code == 200 @@ -74,7 +85,7 @@ def test_search(): assert b"Features" in rv.data -def test_new_file(): +def test_add_new_file(wiki_path): """App can create files.""" rv = app.test_client().get("/add_new") assert rv.status_code == 200 @@ -91,12 +102,53 @@ def test_new_file(): assert b"testing file" in rv.data assert b"this is a test" in rv.data - f = Path(cfg.wiki_directory) / "testing01234filenotexisting.md" + f = wiki_path / "testing01234filenotexisting.md" f.unlink() +def test_bad_file_names(): + """Test for creating files with odd character in names.""" + # Disallowed + bad_name = "file*with*star" + bad_all_bad = r'<>:"/\|?*' + + r = app.test_client().post("/add_new", data={ + "PN": bad_all_bad, + "CT": "#testing file\n this is a test", + }) + assert r.status_code == 200 + assert b"Page name not accepted." in r.data + + r = app.test_client().post("/add_new", data={ + "PN": bad_name, + "CT": "#testing file\n this is a test", + }) + assert r.status_code == 200 + assert bad_name.replace("*", "").encode() in r.data + + +def test_ok_file_names(wiki_path): + """Test for creating files with odd character in names.""" + # Disallowed + ok_name1 = "file with space" + ok_name2 = 'file with slash/is a folder' + r = app.test_client().post("/add_new", data={ + "PN": ok_name1, + "CT": "#testing file\n this is a test", + }) + assert r.status_code == 302 + assert (wiki_path / ok_name1).with_suffix(".md").exists() + + r = app.test_client().post("/add_new", data={ + "PN": ok_name2, + "CT": "#testing file\n this is a test", + }) + assert r.status_code == 302 + assert (wiki_path / ok_name2).with_suffix(".md").exists() + + # create a new file in a folder using the wiki and check if it is visible in the wiki -def test_new_file_folder(): +def test_new_file_folder(wiki_path): """App can create folders.""" rv = app.test_client().get("/add_new") assert rv.status_code == 200 @@ -113,7 +165,7 @@ def test_new_file_folder(): assert b"testing file" in rv.data assert b"this is a test" in rv.data - f = Path(cfg.wiki_directory) / "testingfolder01234" + f = wiki_path / "testingfolder01234" shutil.rmtree(f)