From 5a2057560a5703a59408009e40b51de5a0c18600 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Mon, 18 Jul 2022 18:46:10 +0200 Subject: implement image upload --- .gitignore | 1 + development.ini | 3 +- fietsboek/__init__.py | 11 +++ fietsboek/data.py | 118 +++++++++++++++++++++++++++++++ fietsboek/routes.py | 2 + fietsboek/static/fietsboek.js | 53 ++++++++++++++ fietsboek/static/theme.css | 36 ++++++++++ fietsboek/templates/details.jinja2 | 21 ++++++ fietsboek/templates/edit.jinja2 | 4 +- fietsboek/templates/edit_form.jinja2 | 22 +++++- fietsboek/templates/finish_upload.jinja2 | 4 +- fietsboek/util.py | 69 ++++++++++++++++++ fietsboek/views/detail.py | 30 +++++++- fietsboek/views/edit.py | 17 +++++ fietsboek/views/upload.py | 6 ++ 15 files changed, 389 insertions(+), 8 deletions(-) create mode 100644 fietsboek/data.py diff --git a/.gitignore b/.gitignore index 3afaa13..a833b62 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,4 @@ coverage test *.sqlite .venv/ +/data diff --git a/development.ini b/development.ini index 47e0231..eb33586 100644 --- a/development.ini +++ b/development.ini @@ -18,11 +18,12 @@ pyramid.includes = pyramid_debugtoolbar sqlalchemy.url = sqlite:///%(here)s/fietsboek.sqlite +fietsboek.data_dir = %(here)s/data retry.attempts = 3 email.from = fietsboek@kingdread.de -email.smtp_url = debug://localhost:1025 +email.smtp_url = debug:// available_locales = en de enable_account_registration = true diff --git a/fietsboek/__init__.py b/fietsboek/__init__.py index 4a28dd4..bd2d705 100644 --- a/fietsboek/__init__.py +++ b/fietsboek/__init__.py @@ -2,6 +2,8 @@ For more information, see the README or the included documentation. """ +from pathlib import Path + from pyramid.config import Configurator from pyramid.session import SignedCookieSessionFactory from pyramid.csrf import CookieCSRFStoragePolicy @@ -9,6 +11,7 @@ from pyramid.settings import asbool, aslist from pyramid.i18n import default_locale_negotiator from .security import SecurityPolicy +from .data import DataManager from . import jinja2 as fiets_jinja2 @@ -46,6 +49,13 @@ def main(global_config, **settings): if settings.get('session_key', '') == '': raise ValueError("Please set a session signing key (session_key) in your settings!") + if 'fietsboek.data_dir' not in settings: + raise ValueError("Please set a data directory (fietsboek.data_dir) in your settings!") + + def data_manager(request): + data_dir = request.registry.settings["fietsboek.data_dir"] + return DataManager(Path(data_dir)) + settings['enable_account_registration'] = asbool( settings.get('enable_account_registration', 'false')) settings['available_locales'] = aslist( @@ -63,6 +73,7 @@ def main(global_config, **settings): config.set_csrf_storage_policy(CookieCSRFStoragePolicy()) config.set_default_csrf_options(require_csrf=True) config.set_locale_negotiator(locale_negotiator) + config.add_request_method(data_manager, reify=True) jinja2_env = config.get_jinja2_environment() jinja2_env.filters['format_decimal'] = fiets_jinja2.filter_format_decimal diff --git a/fietsboek/data.py b/fietsboek/data.py new file mode 100644 index 0000000..992ddf7 --- /dev/null +++ b/fietsboek/data.py @@ -0,0 +1,118 @@ +"""Data manager for fietsboek. + +Data are objects that belong to a track (such as images), but are not stored in +the database itself. This module makes access to such data objects easier. +""" +import random +import string +import shutil +import uuid + +from .util import secure_filename + + +def generate_filename(filename): + """Generates a safe-to-use filename for uploads. + + If possible, tries to keep parts of the original filename intact, such as + the extension. + + :param filename: The original filename. + :type filename: str + :return: The generated filename. + :rtype: str + """ + if filename: + good_name = secure_filename(filename) + if good_name: + random_prefix = "".join(random.choice(string.ascii_lowercase) for _ in range(5)) + return f"{random_prefix}_{good_name}" + + return str(uuid.uuid4()) + + +class DataManager: + """Data manager. + + The data manager is usually provided as ``request.data_manager`` and can be + used to access track's images and other on-disk data. + + :ivar data_dir: Path to the data folder. + :vartype data_dir: pathlib.Path + """ + + def __init__(self, data_dir): + self.data_dir = data_dir + + def _track_data_dir(self, track_id): + return self.data_dir / "tracks" / str(track_id) + + def images(self, track_id): + """Returns a list of images that belong to a track. + + :param track_id: Numerical ID of the track. + :type track_id: int + :return: A list of image IDs. + :rtype: list[str] + """ + image_dir = self._track_data_dir(track_id) / "images" + if not image_dir.exists(): + return [] + images = [] + for image in image_dir.iterdir(): + images.append(image.name) + return images + + def image_path(self, track_id, image_id): + """Returns a path to a saved image. + + :raises FileNotFoundError: If the given image could not be found. + :param track_id: ID of the track. + :type track_id: int + :param image_id: ID of the image. + :type image_id: str + :return: A path pointing to the requested image. + :rtype: pathlib.Path + """ + image = self._track_data_dir(track_id) / "images" / secure_filename(image_id) + if not image.exists(): + raise FileNotFoundError("The requested image does not exist") + return image + + def add_image(self, track_id, image, filename=None): + """Saves an image to a track. + + :param track_id: ID of the track. + :type track_id: int + :param image: The image, as a file-like object to read from. + :type image: file + :param filename: The image's original filename. + :type filename: str + :return: The ID of the saved image. + :rtype: str + """ + image_dir = self._track_data_dir(track_id) / "images" + image_dir.mkdir(parents=True, exist_ok=True) + + filename = generate_filename(filename) + path = image_dir / filename + with open(path, "wb") as fobj: + shutil.copyfileobj(image, fobj) + + return filename + + def delete_image(self, track_id, image_id): + """Deletes an image from a track. + + :raises FileNotFoundError: If the given image could not be found. + :param track_id: ID of the track. + :type track_id: int + :param image_id: ID of the image. + :type image_id: str + """ + # Be sure to not delete anything else than the image file + image_id = secure_filename(image_id) + if '/' in image_id or '\\' in image_id: + return + path = self.image_path(track_id, image_id) + path.unlink() diff --git a/fietsboek/routes.py b/fietsboek/routes.py index a28f926..9d705c1 100644 --- a/fietsboek/routes.py +++ b/fietsboek/routes.py @@ -35,6 +35,8 @@ def includeme(config): factory='fietsboek.models.Track.factory') config.add_route('add-comment', '/track/{track_id}/comment', factory='fietsboek.models.Track.factory') + config.add_route('image', '/track/{track_id}/images/{image_name}', + factory='fietsboek.models.Track.factory') config.add_route('badge', '/badge/{badge_id}', factory='fietsboek.models.Badge.factory') diff --git a/fietsboek/static/fietsboek.js b/fietsboek/static/fietsboek.js index d27e21e..e403628 100644 --- a/fietsboek/static/fietsboek.js +++ b/fietsboek/static/fietsboek.js @@ -69,6 +69,48 @@ function removeFriendClicked(event) { button.parentNode.parentNode.removeChild(button.parentNode); } +function imageSelectorChanged(event) { + console.log(event.target.files); + + for (var file of event.target.files) { + const input = document.createElement("input"); + input.type = "file"; + input.hidden = true; + input.name = "image[]"; + + const transfer = new DataTransfer(); + transfer.items.add(file); + input.files = transfer.files; + + let preview = document.querySelector("#trackImagePreviewBlueprint").cloneNode(true); + preview.removeAttribute("id"); + preview.querySelector("img").src = URL.createObjectURL(file); + preview.querySelector("button.delete-image").addEventListener("click", deleteImageButtonClicked); + preview.appendChild(input); + + document.querySelector("#trackImageList").appendChild(preview); + } + + event.target.value = ""; +} + +function deleteImageButtonClicked(event) { + let preview = event.target.closest("div.track-image-preview"); + /* If this was a image yet to be uploaded, simply remove it */ + let input = preview.querySelector("input[type=file]"); + if (input) { + preview.parentNode.removeChild(preview); + return; + } + + /* Otherwise, we need to remove it but also insert a "delete-image" input */ + let deleter = preview.querySelector("input[type=hidden]"); + deleter.removeAttribute("disabled"); + preview.removeChild(deleter); + preview.parentNode.appendChild(deleter); + preview.parentNode.removeChild(preview); +} + document.addEventListener('DOMContentLoaded', function(event) { /* Enable the "Add tag" button in the track edit page */ let $ = (selector) => document.querySelector(selector); @@ -150,4 +192,15 @@ document.addEventListener('DOMContentLoaded', function(event) { $("#archiveDownloadButton").disabled = (checked.length == 0); }); }); + + /* Enable the image selector */ + var button = $("#imageSelector"); + if (button) { + button.addEventListener("change", imageSelectorChanged); + } + + /* Enable the "delete image" buttons for the already existing images */ + document.querySelectorAll("button.delete-image").forEach((b) => { + b.addEventListener("click", deleteImageButtonClicked); + }); }); diff --git a/fietsboek/static/theme.css b/fietsboek/static/theme.css index 831b1f6..076b519 100644 --- a/fietsboek/static/theme.css +++ b/fietsboek/static/theme.css @@ -18,6 +18,42 @@ strong { align-items: center; } +.carousel-item img { + max-height: 700px; + margin: auto; +} + +#trackImageList { + display: flex; + flex-wrap: wrap; +} + +.track-image-preview button { + position: absolute; + z-index: 5; + background-color: white; + right: 0px; +} + +.track-image-preview img { + max-width: 100%; + max-height: 100%; + position: absolute; + top: 50%; + left: 50%; + transform: translate(-50%, -50%); +} + +.track-image-preview { + position: relative; + width: min(317px, 100%); + height: calc(9 / 16 * 317px); + border-radius: 5px; + border: 1px solid grey; + margin-right: 5px; + margin-bottom: 5px; +} + .track-description img { max-width: 100%; } diff --git a/fietsboek/templates/details.jinja2 b/fietsboek/templates/details.jinja2 index 4d88224..2ec62ec 100644 --- a/fietsboek/templates/details.jinja2 +++ b/fietsboek/templates/details.jinja2 @@ -137,6 +137,27 @@
{% endif %} + {% if images %} + + {% endif %}

{{ _("page.details.comments") }}

{% for comment in track.comments %}
diff --git a/fietsboek/templates/edit.jinja2 b/fietsboek/templates/edit.jinja2 index 6053509..4e64f69 100644 --- a/fietsboek/templates/edit.jinja2 +++ b/fietsboek/templates/edit.jinja2 @@ -9,8 +9,8 @@
-
- {{ edit_form.edit_track(track.title, track.date_raw, track.date_tz or 0, track.visibility, track.description, track.text_tags(), badges, track.tagged_people) }} + + {{ edit_form.edit_track(track.title, track.date_raw, track.date_tz or 0, track.visibility, track.description, track.text_tags(), badges, track.tagged_people, images) }} {{ util.hidden_csrf_input() }}
diff --git a/fietsboek/templates/edit_form.jinja2 b/fietsboek/templates/edit_form.jinja2 index 3779d5d..0ba933e 100644 --- a/fietsboek/templates/edit_form.jinja2 +++ b/fietsboek/templates/edit_form.jinja2 @@ -1,4 +1,4 @@ -{% macro edit_track(title, date, date_tz, visibility, description, tags, badges, friends) %} +{% macro edit_track(title, date, date_tz, visibility, description, tags, badges, friends, images) %}
@@ -87,4 +87,24 @@
+
+
+ {% for image in images %} +
+ + + +
+ {% endfor %} +
+ + +
+ +
+
+ + +
+
{% endmacro %} diff --git a/fietsboek/templates/finish_upload.jinja2 b/fietsboek/templates/finish_upload.jinja2 index 58c67d4..d6f9f00 100644 --- a/fietsboek/templates/finish_upload.jinja2 +++ b/fietsboek/templates/finish_upload.jinja2 @@ -9,8 +9,8 @@
- - {{ edit_form.edit_track(upload_title, upload_date, upload_date_tz, upload_visibility, upload_description, upload_tags, badges, upload_tagged_people) }} + + {{ edit_form.edit_track(upload_title, upload_date, upload_date_tz, upload_visibility, upload_description, upload_tags, badges, upload_tagged_people, []) }} {{ util.hidden_csrf_input() }}
diff --git a/fietsboek/util.py b/fietsboek/util.py index 1014c87..bab2074 100644 --- a/fietsboek/util.py +++ b/fietsboek/util.py @@ -2,6 +2,9 @@ import random import string import datetime +import re +import os +import unicodedata # Compat for Python < 3.9 import importlib_resources @@ -25,6 +28,22 @@ ALLOWED_ATTRIBUTES = dict(bleach.sanitizer.ALLOWED_ATTRIBUTES) ALLOWED_ATTRIBUTES['img'] = ['alt', 'src'] +_filename_ascii_strip_re = re.compile(r"[^A-Za-z0-9_.-]") +_windows_device_files = ( + "CON", + "AUX", + "COM1", + "COM2", + "COM3", + "COM4", + "LPT1", + "LPT2", + "LPT3", + "PRN", + "NUL", +) + + def safe_markdown(md_source): """Transform a markdown document into a safe HTML document. @@ -289,3 +308,53 @@ def read_localized_resource(locale_name, path, raise_on_error=False): if raise_on_error: raise FileNotFoundError(f"Resource {path!r} not found") return f"{locale_name}:{path}" + + +def secure_filename(filename): + r"""Pass it a filename and it will return a secure version of it. This + filename can then safely be stored on a regular file system and passed + to :func:`os.path.join`. The filename returned is an ASCII only string + for maximum portability. + On windows systems the function also makes sure that the file is not + named after one of the special device files. + + >>> secure_filename("My cool movie.mov") + 'My_cool_movie.mov' + >>> secure_filename("../../../etc/passwd") + 'etc_passwd' + >>> secure_filename('i contain cool \xfcml\xe4uts.txt') + 'i_contain_cool_umlauts.txt' + + The function might return an empty filename. It's your responsibility + to ensure that the filename is unique and that you abort or + generate a random filename if the function returned an empty one. + + :param filename: the filename to secure + :type filename: str + :return: The secure filename. + :rtype: str + """ + # Taken from + # https://github.com/pallets/werkzeug/blob/main/src/werkzeug/utils.py + + filename = unicodedata.normalize("NFKD", filename) + filename = filename.encode("ascii", "ignore").decode("ascii") + + for sep in os.path.sep, os.path.altsep: + if sep: + filename = filename.replace(sep, " ") + filename = str(_filename_ascii_strip_re.sub("", "_".join(filename.split()))).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 + if ( + os.name == "nt" + and filename + and filename.split(".", maxsplit=1)[0].upper() in _windows_device_files + ): + filename = f"_{filename}" + + return filename diff --git a/fietsboek/views/detail.py b/fietsboek/views/detail.py index a15b29d..3953f47 100644 --- a/fietsboek/views/detail.py +++ b/fietsboek/views/detail.py @@ -2,9 +2,9 @@ import datetime from pyramid.view import view_config -from pyramid.response import Response +from pyramid.response import Response, FileResponse from pyramid.i18n import TranslationString as _ -from pyramid.httpexceptions import HTTPFound +from pyramid.httpexceptions import HTTPFound, HTTPNotFound from .. import models, util @@ -22,12 +22,17 @@ def details(request): track = request.context description = util.safe_markdown(track.description) show_edit_link = (track.owner == request.identity) + images = [ + request.route_url("image", track_id=track.id, image_name=image) + for image in request.data_manager.images(track.id) + ] return { 'track': track, 'show_edit_link': show_edit_link, 'mps_to_kph': util.mps_to_kph, 'comment_md_to_html': util.safe_markdown, 'description': description, + 'images': images, } @@ -85,6 +90,27 @@ def badge(request): return Response(request.context.image) +@view_config(route_name='image', http_cache=3600, permission='track.view') +def image(request): + """Returns the image data for the requested image. + + This ensures that the image is sent efficiently, by delegating to the WSGI + file wrapper if possible. + + :param request: The Pyramid request. + :type request: pyramid.request.Request + :return: The HTTP response. + :rtype: pyramid.response.Response + """ + track = request.context + try: + image_path = request.data_manager.image_path(track.id, request.matchdict['image_name']) + except FileNotFoundError: + return HTTPNotFound() + else: + return FileResponse(image_path, request) + + @view_config(route_name="add-comment", request_method="POST", permission="track.comment") def add_comment(request): """Endpoint to add a comment to a track. diff --git a/fietsboek/views/edit.py b/fietsboek/views/edit.py index aabbcbc..04ed000 100644 --- a/fietsboek/views/edit.py +++ b/fietsboek/views/edit.py @@ -1,5 +1,6 @@ """Views for editing a track.""" import datetime +from collections import namedtuple from pyramid.view import view_config from pyramid.httpexceptions import HTTPFound, HTTPBadRequest @@ -10,6 +11,9 @@ from .. import models, util from ..models.track import Visibility +_Image = namedtuple("_Image", "name url") + + @view_config(route_name='edit', renderer='fietsboek:templates/edit.jinja2', permission='track.edit', request_method='GET') def edit(request): @@ -23,9 +27,14 @@ def edit(request): track = request.context badges = request.dbsession.execute(select(models.Badge)).scalars() badges = [(badge in track.badges, badge) for badge in badges] + images = [ + _Image(image, request.route_url("image", track_id=track.id, image_name=image)) + for image in request.data_manager.images(track.id) + ] return { 'track': track, 'badges': badges, + 'images': images, } @@ -61,4 +70,12 @@ def do_edit(request): tags = request.params.getall("tag[]") track.sync_tags(tags) + for image in request.params.getall("image[]"): + if image == b"": + continue + request.data_manager.add_image(track.id, image.file, image.filename) + + for image in request.params.getall("delete-image[]"): + request.data_manager.delete_image(track.id, image) + return HTTPFound(request.route_url('details', track_id=track.id)) diff --git a/fietsboek/views/upload.py b/fietsboek/views/upload.py index 979291e..4f42b80 100644 --- a/fietsboek/views/upload.py +++ b/fietsboek/views/upload.py @@ -169,6 +169,12 @@ def do_finish_upload(request): track.ensure_cache() request.dbsession.add(track.cache) + # Don't forget to add the images + for image in request.params.getall("image[]"): + if image == b"": + continue + request.data_manager.add_image(track.id, image.file, image.filename) + request.session.flash(request.localizer.translate(_("flash.upload_success"))) return HTTPFound(request.route_url('details', track_id=track.id)) -- cgit v1.2.3