From 8105d9388b2287db5347f0d0f5bd1c88d723a831 Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 18 Nov 2021 16:49:37 +0100 Subject: [PATCH] :recycle: Refactor rlimit usage (backend). --- backend/src/app/config.clj | 15 +++++--- backend/src/app/main.clj | 19 --------- backend/src/app/media.clj | 18 +++------ backend/src/app/rlimits.clj | 45 ---------------------- backend/src/app/rpc.clj | 33 +++++----------- backend/src/app/rpc/mutations/comments.clj | 2 - backend/src/app/rpc/mutations/fonts.clj | 6 ++- backend/src/app/rpc/mutations/media.clj | 14 ++++--- backend/src/app/rpc/mutations/profile.clj | 18 ++++++--- backend/src/app/rpc/mutations/teams.clj | 24 ++++++------ backend/src/app/util/rlimit.clj | 36 +++++++++++++++++ 11 files changed, 95 insertions(+), 135 deletions(-) delete mode 100644 backend/src/app/rlimits.clj create mode 100644 backend/src/app/util/rlimit.clj diff --git a/backend/src/app/config.clj b/backend/src/app/config.clj index fb8612930..98c003e55 100644 --- a/backend/src/app/config.clj +++ b/backend/src/app/config.clj @@ -62,8 +62,9 @@ :assets-path "/internal/assets/" - :rlimits-password 10 - :rlimits-image 2 + :rlimit-password 10 + :rlimit-image 2 + :rlimit-font 5 :smtp-default-reply-to "Penpot " :smtp-default-from "Penpot " @@ -151,8 +152,9 @@ (s/def ::public-uri ::us/string) (s/def ::redis-uri ::us/string) (s/def ::registration-domain-whitelist ::us/set-of-str) -(s/def ::rlimits-image ::us/integer) -(s/def ::rlimits-password ::us/integer) +(s/def ::rlimit-font ::us/integer) +(s/def ::rlimit-image ::us/integer) +(s/def ::rlimit-password ::us/integer) (s/def ::smtp-default-from ::us/string) (s/def ::smtp-default-reply-to ::us/string) (s/def ::smtp-host ::us/string) @@ -237,8 +239,9 @@ ::redis-uri ::registration-domain-whitelist ::registration-enabled - ::rlimits-image - ::rlimits-password + ::rlimit-font + ::rlimit-image + ::rlimit-password ::sentry-dsn ::sentry-debug ::sentry-attach-stack-trace diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index 456cd4725..068136336 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -127,24 +127,6 @@ :audit (ig/ref :app.loggers.audit/collector) :public-uri (cf/get :public-uri)} - ;; RLimit definition for password hashing - :app.rlimits/password - (cf/get :rlimits-password) - - ;; RLimit definition for image processing - :app.rlimits/image - (cf/get :rlimits-image) - - ;; RLimit definition for font processing - :app.rlimits/font - (cf/get :rlimits-font 2) - - ;; A collection of rlimits as hash-map. - :app.rlimits/all - {:password (ig/ref :app.rlimits/password) - :image (ig/ref :app.rlimits/image) - :font (ig/ref :app.rlimits/font)} - :app.rpc/rpc {:pool (ig/ref :app.db/pool) :session (ig/ref :app.http.session/session) @@ -152,7 +134,6 @@ :metrics (ig/ref :app.metrics/metrics) :storage (ig/ref :app.storage/storage) :msgbus (ig/ref :app.msgbus/msgbus) - :rlimits (ig/ref :app.rlimits/all) :public-uri (cf/get :public-uri) :audit (ig/ref :app.loggers.audit/collector)} diff --git a/backend/src/app/media.clj b/backend/src/app/media.clj index 22075a643..0142638e7 100644 --- a/backend/src/app/media.clj +++ b/backend/src/app/media.clj @@ -12,7 +12,6 @@ [app.common.media :as cm] [app.common.spec :as us] [app.config :as cf] - [app.rlimits :as rlm] [app.util.svg :as svg] [buddy.core.bytes :as bb] [buddy.core.codecs :as bc] @@ -51,7 +50,6 @@ :code :media-type-not-allowed :hint "Seems like you are uploading an invalid media object")))) - (defmulti process :cmd) (defmulti process-error class) @@ -66,17 +64,11 @@ (throw error)) (defn run - [{:keys [rlimits] :as cfg} {:keys [rlimit] :or {rlimit :image} :as params}] - (us/assert map? rlimits) - (let [rlimit (get rlimits rlimit)] - (when-not rlimit - (ex/raise :type :internal - :code :rlimit-not-configured - :hint ":image rlimit not configured")) - (try - (rlm/execute rlimit (process params)) - (catch Throwable e - (process-error e))))) + [params] + (try + (process params) + (catch Throwable e + (process-error e)))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; --- Thumbnails Generation diff --git a/backend/src/app/rlimits.clj b/backend/src/app/rlimits.clj deleted file mode 100644 index aaf813178..000000000 --- a/backend/src/app/rlimits.clj +++ /dev/null @@ -1,45 +0,0 @@ -;; This Source Code Form is subject to the terms of the Mozilla Public -;; License, v. 2.0. If a copy of the MPL was not distributed with this -;; file, You can obtain one at http://mozilla.org/MPL/2.0/. -;; -;; Copyright (c) UXBOX Labs SL - -(ns app.rlimits - "Resource usage limits (in other words: semaphores)." - (:require - [app.common.spec :as us] - [clojure.spec.alpha :as s] - [integrant.core :as ig]) - (:import - java.util.concurrent.Semaphore)) - -(s/def ::rlimit #(instance? Semaphore %)) -(s/def ::rlimits (s/map-of ::us/keyword ::rlimit)) - -(derive ::password ::instance) -(derive ::image ::instance) -(derive ::font ::instance) - -(defmethod ig/pre-init-spec ::instance [_] - (s/spec int?)) - -(defmethod ig/init-key ::instance - [_ permits] - (Semaphore. (int permits))) - -(defn acquire! - [sem] - (.acquire ^Semaphore sem)) - -(defn release! - [sem] - (.release ^Semaphore sem)) - -(defmacro execute - [rlinst & body] - `(try - (acquire! ~rlinst) - ~@body - (finally - (release! ~rlinst)))) - diff --git a/backend/src/app/rpc.clj b/backend/src/app/rpc.clj index 13c9089cc..ebd026670 100644 --- a/backend/src/app/rpc.clj +++ b/backend/src/app/rpc.clj @@ -13,11 +13,10 @@ [app.db :as db] [app.loggers.audit :as audit] [app.metrics :as mtx] - [app.rlimits :as rlm] [app.util.retry :as retry] + [app.util.rlimit :as rlimit] [app.util.services :as sv] [clojure.spec.alpha :as s] - [cuerdas.core :as str] [integrant.core :as ig])) (defn- default-handler @@ -74,29 +73,15 @@ [cfg f mdata] (mtx/wrap-summary f (::mobj cfg) [(::sv/name mdata)])) -;; Wrap the rpc handler with a semaphore if it is specified in the -;; metadata asocciated with the handler. -(defn- wrap-with-rlimits - [cfg f mdata] - (if-let [key (:rlimit mdata)] - (let [rlinst (get-in cfg [:rlimits key])] - (when-not rlinst - (ex/raise :type :internal - :code :rlimit-not-configured - :hint (str/fmt "%s rlimit not configured" key))) - (l/trace :action "add rlimit" - :handler (::sv/name mdata)) - (fn [cfg params] - (rlm/execute rlinst (f cfg params)))) - f)) - (defn- wrap-impl [{:keys [audit] :as cfg} f mdata] - (let [f (wrap-with-rlimits cfg f mdata) - f (retry/wrap-retry cfg f mdata) - f (wrap-with-metrics cfg f mdata) - spec (or (::sv/spec mdata) (s/spec any?)) - auth? (:auth mdata true)] + (let [f (as-> f $ + (rlimit/wrap-rlimit cfg $ mdata) + (retry/wrap-retry cfg $ mdata) + (wrap-with-metrics cfg $ mdata)) + + spec (or (::sv/spec mdata) (s/spec any?)) + auth? (:auth mdata true)] (l/trace :action "register" :name (::sv/name mdata)) (with-meta @@ -188,7 +173,7 @@ (defmethod ig/pre-init-spec ::rpc [_] (s/keys :req-un [::storage ::session ::tokens ::audit - ::mtx/metrics ::rlm/rlimits ::db/pool])) + ::mtx/metrics ::db/pool])) (defmethod ig/init-key ::rpc [_ cfg] diff --git a/backend/src/app/rpc/mutations/comments.clj b/backend/src/app/rpc/mutations/comments.clj index 3a6abab4b..ba94040d8 100644 --- a/backend/src/app/rpc/mutations/comments.clj +++ b/backend/src/app/rpc/mutations/comments.clj @@ -12,9 +12,7 @@ [app.rpc.queries.comments :as comments] [app.rpc.queries.files :as files] [app.util.blob :as blob] - #_:clj-kondo/ignore [app.util.retry :as retry] - [app.util.services :as sv] [app.util.time :as dt] [clojure.spec.alpha :as s])) diff --git a/backend/src/app/rpc/mutations/fonts.clj b/backend/src/app/rpc/mutations/fonts.clj index b9682dad2..6416567d0 100644 --- a/backend/src/app/rpc/mutations/fonts.clj +++ b/backend/src/app/rpc/mutations/fonts.clj @@ -9,10 +9,12 @@ [app.common.exceptions :as ex] [app.common.spec :as us] [app.common.uuid :as uuid] + [app.config :as cf] [app.db :as db] [app.media :as media] [app.rpc.queries.teams :as teams] [app.storage :as sto] + [app.util.rlimit :as rlimit] [app.util.services :as sv] [app.util.time :as dt] [clojure.spec.alpha :as s])) @@ -37,6 +39,7 @@ ::font-id ::font-family ::font-weight ::font-style])) (sv/defmethod ::create-font-variant + {::rlimit/permits (cf/get :rlimit-font)} [{:keys [pool] :as cfg} {:keys [team-id profile-id] :as params}] (db/with-atomic [conn pool] (let [cfg (assoc cfg :conn conn)] @@ -45,10 +48,9 @@ (defn create-font-variant [{:keys [conn storage] :as cfg} {:keys [data] :as params}] - (let [data (media/run cfg {:cmd :generate-fonts :input data :rlimit :font}) + (let [data (media/run {:cmd :generate-fonts :input data}) storage (media/configure-assets-storage storage conn) - otf (when-let [fdata (get data "font/otf")] (sto/put-object storage {:content (sto/content fdata) :content-type "font/otf"})) diff --git a/backend/src/app/rpc/mutations/media.clj b/backend/src/app/rpc/mutations/media.clj index bfe876df1..90e5e0c14 100644 --- a/backend/src/app/rpc/mutations/media.clj +++ b/backend/src/app/rpc/mutations/media.clj @@ -10,11 +10,13 @@ [app.common.media :as cm] [app.common.spec :as us] [app.common.uuid :as uuid] + [app.config :as cf] [app.db :as db] [app.media :as media] [app.rpc.queries.teams :as teams] [app.storage :as sto] [app.util.http :as http] + [app.util.rlimit :as rlimit] [app.util.services :as sv] [app.util.time :as dt] [clojure.spec.alpha :as s] @@ -47,6 +49,7 @@ :opt-un [::id])) (sv/defmethod ::upload-file-media-object + {::rlimit/permits (cf/get :rlimit-image)} [{:keys [pool] :as cfg} {:keys [profile-id file-id] :as params}] (db/with-atomic [conn pool] (let [file (select-file conn file-id)] @@ -89,21 +92,20 @@ :content-type mtype :expired-at (dt/in-future {:minutes 30})})))) - (defn create-file-media-object [{:keys [conn storage] :as cfg} {:keys [id file-id is-local name content] :as params}] (media/validate-media-type (:content-type content)) (let [storage (media/configure-assets-storage storage conn) source-path (fs/path (:tempfile content)) source-mtype (:content-type content) - source-info (media/run cfg {:cmd :info :input {:path source-path :mtype source-mtype}}) + source-info (media/run {:cmd :info :input {:path source-path :mtype source-mtype}}) thumb (when (and (not (svg-image? source-info)) (big-enough-for-thumbnail? source-info)) - (media/run cfg (assoc thumbnail-options - :cmd :generic-thumbnail - :input {:mtype (:mtype source-info) - :path source-path}))) + (media/run (assoc thumbnail-options + :cmd :generic-thumbnail + :input {:mtype (:mtype source-info) + :path source-path}))) image (if (= (:mtype source-info) "image/svg+xml") (let [data (slurp source-path)] diff --git a/backend/src/app/rpc/mutations/profile.clj b/backend/src/app/rpc/mutations/profile.clj index 45c2bb5bd..fb53e9d94 100644 --- a/backend/src/app/rpc/mutations/profile.clj +++ b/backend/src/app/rpc/mutations/profile.clj @@ -19,6 +19,7 @@ [app.rpc.mutations.teams :as teams] [app.rpc.queries.profile :as profile] [app.storage :as sto] + [app.util.rlimit :as rlimit] [app.util.services :as sv] [app.util.time :as dt] [buddy.hashers :as hashers] @@ -128,7 +129,8 @@ (s/def ::register-profile (s/keys :req-un [::token ::fullname])) -(sv/defmethod ::register-profile {:auth false :rlimit :password} +(sv/defmethod ::register-profile + {:auth false ::rlimit/permits (cf/get :rlimit-password)} [{:keys [pool] :as cfg} params] (db/with-atomic [conn pool] (-> (assoc cfg :conn conn) @@ -281,7 +283,8 @@ (s/keys :req-un [::email ::password] :opt-un [::scope ::invitation-token])) -(sv/defmethod ::login {:auth false :rlimit :password} +(sv/defmethod ::login + {:auth false ::rlimit/permits (cf/get :rlimit-password)} [{:keys [pool session tokens] :as cfg} {:keys [email password] :as params}] (letfn [(check-password [profile password] (when (= (:password profile) "!") @@ -371,7 +374,8 @@ (s/def ::update-profile-password (s/keys :req-un [::profile-id ::password ::old-password])) -(sv/defmethod ::update-profile-password {:rlimit :password} +(sv/defmethod ::update-profile-password + {::rlimit/permits (cf/get :rlimit-password)} [{:keys [pool] :as cfg} {:keys [password] :as params}] (db/with-atomic [conn pool] (let [profile (validate-password! conn params)] @@ -404,11 +408,12 @@ (s/keys :req-un [::profile-id ::file])) (sv/defmethod ::update-profile-photo + {::rlimit/permits (cf/get :rlimit-image)} [{:keys [pool storage] :as cfg} {:keys [profile-id file] :as params}] (db/with-atomic [conn pool] (media/validate-media-type (:content-type file) #{"image/jpeg" "image/png" "image/webp"}) - (media/run cfg {:cmd :info :input {:path (:tempfile file) - :mtype (:content-type file)}}) + (media/run {:cmd :info :input {:path (:tempfile file) + :mtype (:content-type file)}}) (let [profile (db/get-by-id conn :profile profile-id) storage (media/configure-assets-storage storage conn) @@ -554,7 +559,8 @@ (s/def ::recover-profile (s/keys :req-un [::token ::password])) -(sv/defmethod ::recover-profile {:auth false :rlimit :password} +(sv/defmethod ::recover-profile + {:auth false ::rlimit/permits (cf/get :rlimit-password)} [{:keys [pool tokens] :as cfg} {:keys [token password]}] (letfn [(validate-token [token] (let [tdata (tokens :verify {:token token :iss :password-recovery})] diff --git a/backend/src/app/rpc/mutations/teams.clj b/backend/src/app/rpc/mutations/teams.clj index 0f44afb17..7f2254f3c 100644 --- a/backend/src/app/rpc/mutations/teams.clj +++ b/backend/src/app/rpc/mutations/teams.clj @@ -10,6 +10,7 @@ [app.common.exceptions :as ex] [app.common.spec :as us] [app.common.uuid :as uuid] + [app.config :as cf] [app.db :as db] [app.emails :as eml] [app.media :as media] @@ -18,6 +19,7 @@ [app.rpc.queries.profile :as profile] [app.rpc.queries.teams :as teams] [app.storage :as sto] + [app.util.rlimit :as rlimit] [app.util.services :as sv] [app.util.time :as dt] [clojure.spec.alpha :as s] @@ -259,12 +261,13 @@ (s/keys :req-un [::profile-id ::team-id ::file])) (sv/defmethod ::update-team-photo + {::rlimit/permits (cf/get :rlimit-image)} [{:keys [pool storage] :as cfg} {:keys [profile-id file team-id] :as params}] (db/with-atomic [conn pool] (teams/check-edition-permissions! conn profile-id team-id) (media/validate-media-type (:content-type file) #{"image/jpeg" "image/png" "image/webp"}) - (media/run cfg {:cmd :info :input {:path (:tempfile file) - :mtype (:content-type file)}}) + (media/run {:cmd :info :input {:path (:tempfile file) + :mtype (:content-type file)}}) (let [team (teams/retrieve-team conn profile-id team-id) storage (media/configure-assets-storage storage conn) @@ -284,16 +287,13 @@ (defn upload-photo [{:keys [storage] :as cfg} {:keys [file]}] - (let [thumb (media/run cfg - {:cmd :profile-thumbnail - :format :jpeg - :quality 85 - :width 256 - :height 256 - :input {:path (fs/path (:tempfile file)) - :mtype (:content-type file)}})] - - + (let [thumb (media/run {:cmd :profile-thumbnail + :format :jpeg + :quality 85 + :width 256 + :height 256 + :input {:path (fs/path (:tempfile file)) + :mtype (:content-type file)}})] (sto/put-object storage {:content (sto/content (:data thumb) (:size thumb)) :content-type (:mtype thumb)}))) diff --git a/backend/src/app/util/rlimit.clj b/backend/src/app/util/rlimit.clj new file mode 100644 index 000000000..8398237c1 --- /dev/null +++ b/backend/src/app/util/rlimit.clj @@ -0,0 +1,36 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) UXBOX Labs SL + +(ns app.util.rlimit + "Resource usage limits (in other words: semaphores)." + (:require + [app.common.logging :as l] + [app.util.services :as sv]) + (:import + java.util.concurrent.Semaphore)) + +(defn acquire! + [sem] + (.acquire ^Semaphore sem)) + +(defn release! + [sem] + (.release ^Semaphore sem)) + +(defn wrap-rlimit + [_cfg f mdata] + (if-let [permits (::permits mdata)] + (let [sem (Semaphore. permits)] + (l/debug :hint "wrapping rlimit" :handler (::sv/name mdata) :permits permits) + (fn [cfg params] + (try + (acquire! sem) + (f cfg params) + (finally + (release! sem))))) + f)) + +