From facdd35b11d3a635246a6287ea33d020f64c349b Mon Sep 17 00:00:00 2001 From: rubenwardy Date: Sun, 19 Jan 2020 01:37:15 +0000 Subject: [PATCH] Add validation to zip releases --- app/blueprints/packages/releases.py | 15 ++++++----- app/blueprints/packages/screenshots.py | 6 ++--- app/tasks/importtasks.py | 35 +++++++++++++++++++++++++- app/utils.py | 11 ++++---- 4 files changed, 52 insertions(+), 15 deletions(-) diff --git a/app/blueprints/packages/releases.py b/app/blueprints/packages/releases.py index 6ef19d20..e160fcf7 100644 --- a/app/blueprints/packages/releases.py +++ b/app/blueprints/packages/releases.py @@ -22,7 +22,7 @@ from . import bp from app.rediscache import has_key, set_key, make_download_key from app.models import * -from app.tasks.importtasks import makeVCSRelease +from app.tasks.importtasks import makeVCSRelease, checkZIPRelease from app.utils import * from celery import uuid @@ -98,22 +98,25 @@ def create_release(package): return redirect(url_for("tasks.check", id=rel.task_id, r=rel.getEditURL())) else: - uploadedPath = doFileUpload(form.fileUpload.data, "zip", "a zip file") - if uploadedPath is not None: + uploadedUrl, uploadedPath = doFileUpload(form.fileUpload.data, "zip", "a zip file") + if uploadedUrl is not None: rel = PackageRelease() rel.package = package rel.title = form["title"].data - rel.url = uploadedPath + rel.url = uploadedUrl + rel.task_id = uuid() rel.min_rel = form["min_rel"].data.getActual() rel.max_rel = form["max_rel"].data.getActual() - rel.approve(current_user) db.session.add(rel) db.session.commit() + checkZIPRelease.apply_async((rel.id, uploadedPath), task_id=rel.task_id) + msg = "{}: Release {} created".format(package.title, rel.title) triggerNotif(package.author, current_user, msg, rel.getEditURL()) db.session.commit() - return redirect(package.getDetailsURL()) + + return redirect(url_for("tasks.check", id=rel.task_id, r=rel.getEditURL())) return render_template("packages/release_new.html", package=package, form=form) diff --git a/app/blueprints/packages/screenshots.py b/app/blueprints/packages/screenshots.py index c7fc7eb4..2646e292 100644 --- a/app/blueprints/packages/screenshots.py +++ b/app/blueprints/packages/screenshots.py @@ -50,13 +50,13 @@ def create_screenshot(package, id=None): # Initial form class from post data and default data form = CreateScreenshotForm() if request.method == "POST" and form.validate(): - uploadedPath = doFileUpload(form.fileUpload.data, "image", + uploadedUrl, uploadedPath = doFileUpload(form.fileUpload.data, "image", "a PNG or JPG image file") - if uploadedPath is not None: + if uploadedUrl is not None: ss = PackageScreenshot() ss.package = package ss.title = form["title"].data or "Untitled" - ss.url = uploadedPath + ss.url = uploadedUrl ss.approved = package.checkPerm(current_user, Permission.APPROVE_SCREENSHOT) db.session.add(ss) diff --git a/app/tasks/importtasks.py b/app/tasks/importtasks.py index 7084909f..3c0b0edd 100644 --- a/app/tasks/importtasks.py +++ b/app/tasks/importtasks.py @@ -22,6 +22,8 @@ from flask_sqlalchemy import SQLAlchemy from urllib.error import HTTPError import urllib.request from urllib.parse import urlparse, quote_plus, urlsplit +from zipfile import ZipFile + from app import app from app.models import * from app.tasks import celery, TaskError @@ -133,12 +135,17 @@ def generateGitURL(urlstr): return "http://:@" + netloc + path + query + +def getTempDir(): + return os.path.join(tempfile.gettempdir(), randomString(10)) + + # Clones a repo from an unvalidated URL. # Returns a tuple of path and repo on sucess. # Throws `TaskError` on failure. # Caller is responsible for deleting returned directory. def cloneRepo(urlstr, ref=None, recursive=False): - gitDir = tempfile.gettempdir() + "/" + randomString(10) + gitDir = getTempDir() err = None try: @@ -222,6 +229,32 @@ def makeVCSReleaseFromGithub(id, branch, release, url): return release.url +@celery.task() +def checkZIPRelease(id, path): + release = PackageRelease.query.get(id) + if release is None: + raise TaskError("No such release!") + elif release.package is None: + raise TaskError("No package attached to release") + + temp = getTempDir() + try: + with ZipFile(path, 'r') as zip_ref: + zip_ref.extractall(temp) + + try: + tree = build_tree(temp, expected_type=ContentType[release.package.type.name], \ + author=release.package.author.username, name=release.package.name) + except MinetestCheckError as err: + raise TaskError(str(err)) + + release.task_id = None + release.approve(release.package.author) + db.session.commit() + + finally: + shutil.rmtree(temp) + @celery.task() def makeVCSRelease(id, branch): diff --git a/app/utils.py b/app/utils.py index 983111ca..fa423cbe 100644 --- a/app/utils.py +++ b/app/utils.py @@ -49,7 +49,7 @@ def randomString(n): def doFileUpload(file, fileType, fileTypeDesc): if not file or file is None or file.filename == "": flash("No selected file", "error") - return None + return None, None assert os.path.isdir(app.config["UPLOAD_DIR"]), "UPLOAD_DIR must exist" @@ -66,17 +66,18 @@ def doFileUpload(file, fileType, fileTypeDesc): ext = getExtension(file.filename) if ext is None or not ext in allowedExtensions: flash("Please upload load " + fileTypeDesc, "danger") - return None + return None, None if isImage and not isAllowedImage(file.stream.read()): flash("Uploaded image isn't actually an image", "danger") - return None + return None, None file.stream.seek(0) filename = randomString(10) + "." + ext - file.save(os.path.join(app.config["UPLOAD_DIR"], filename)) - return "/uploads/" + filename + filepath = os.path.join(app.config["UPLOAD_DIR"], filename) + file.save(filepath) + return "/uploads/" + filename, filepath def make_flask_user_password(plaintext_str): # http://passlib.readthedocs.io/en/stable/modular_crypt_format.html