♻️ Refactor rlimit usage (backend).

2025-01-08 16:00:19 -05:00 · 2021-11-18 16:49:37 +01:00 · 2021-11-18 16:49:37 +01:00 · 8105d9388b
commit 8105d9388b
parent 8151dcc05f
11 changed files with 95 additions and 135 deletions
--- 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 <no-reply@example.com>"
   :smtp-default-from "Penpot <no-reply@example.com>"
@ -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
--- 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)}

--- 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
--- a/backend/src/app/rlimits.clj
+++ b/backend/src/app/rlimits.clj
@ -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))))
-
--- 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]
--- 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]))
--- 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"}))
--- 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)]
--- 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})]
--- 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)})))
--- a/backend/src/app/util/rlimit.clj
+++ 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))
+
+