From 510bfb354504c53450a753a26c58796ad049cd1b Mon Sep 17 00:00:00 2001 From: Phil Hagelberg Date: Thu, 9 May 2013 14:20:24 -0700 Subject: [PATCH] Move validation into ev/record-deploy. --- project.clj | 1 + src/clojars/db.clj | 34 +------------------- src/clojars/event.clj | 30 +++++++++++++++++- src/clojars/routes/repo.clj | 12 +++---- src/clojars/scp.clj | 42 +++++++++++++------------ test/clojars/test/integration/scp.clj | 13 ++++---- test/clojars/test/integration/steps.clj | 2 +- test/clojars/test/unit/db.clj | 17 ---------- 8 files changed, 67 insertions(+), 84 deletions(-) diff --git a/project.clj b/project.clj index 8ba52295..29d872bd 100644 --- a/project.clj +++ b/project.clj @@ -34,6 +34,7 @@ [nailgun-shim "0.0.1"]]} :dev {:dependencies [[kerodon "0.0.7"] [nailgun-shim "0.0.1"]] + ;; TODO: set test resources in fixture w/ with-redefs :resource-paths ["local-resources"]}} :plugins [[lein-ring "0.7.3" :exclusions [thneed]] ;fix downloading -snapshot all the time diff --git a/src/clojars/db.clj b/src/clojars/db.clj index a2124072..297cd025 100644 --- a/src/clojars/db.clj +++ b/src/clojars/db.clj @@ -281,14 +281,9 @@ (throw (Exception. (str "You don't have access to the " groupname " group."))))))) -(defn- prevent-redeploy [group name version] - (when (.exists (io/file (:repo config) group name version)) - (throw (ex-info "Redeploying is not allowed." {:status 403})))) - -(defn- add-jar-helper [account {:keys [group name version +(defn add-jar [account {:keys [group name version description homepage authors]}] (check-and-add-group account group) - (prevent-redeploy group name version) (insert jars (values {:group_name group :jar_name name @@ -299,30 +294,3 @@ :homepage homepage :authors (str/join ", " (map #(.replace % "," "") authors))}))) - -(defn- validate [x re message] - (when-not (re-matches re x) - (throw (Exception. (str message " (" re ")"))))) - -(defn add-jar [account jarmap & [check-only]] - ;; We're on purpose *at least* as restrictive as the recommendations on - ;; https://maven.apache.org/guides/mini/guide-naming-conventions.html - ;; If you want loosen these please include in your proposal the - ;; ramifications on usability, security and compatiblity with filesystems, - ;; OSes, URLs and tools. - (validate (:name jarmap) #"^[a-z0-9_.-]+$" - (str "Jar names must consist solely of lowercase " - "letters, numbers, hyphens and underscores.")) - ;; Maven's pretty accepting of version numbers, but so far in 2.5 years - ;; bar one broken non-ascii exception only these characters have been used. - ;; Even if we manage to support obscure characters some filesystems do not - ;; and some tools fail to escape URLs properly. So to keep things nice and - ;; compatible for everyone let's lock it down. - (validate (:version jarmap) #"^[a-zA-Z0-9_.+-]+$" - (str "Version strings must consist solely of letters, " - "numbers, dots, pluses, hyphens and underscores.")) - (transaction - (if check-only - (do (rollback) - (add-jar-helper account jarmap)) - (add-jar-helper account jarmap)))) diff --git a/src/clojars/event.clj b/src/clojars/event.clj index 746543e5..767fc125 100644 --- a/src/clojars/event.clj +++ b/src/clojars/event.clj @@ -8,6 +8,33 @@ [clucy.core :as clucy]) (:import (org.apache.commons.codec.digest DigestUtils))) +(defn- validate-regex [x re message] + (when-not (re-matches re x) + (throw (Exception. (str message " (" re ")"))))) + +(defn- validate-deploy [group-id artifact-id version] + ;; We're on purpose *at least* as restrictive as the recommendations on + ;; https://maven.apache.org/guides/mini/guide-naming-conventions.html + ;; If you want loosen these please include in your proposal the + ;; ramifications on usability, security and compatiblity with filesystems, + ;; OSes, URLs and tools. + (validate-regex artifact-id #"^[a-z0-9_.-]+$" + (str "Jar names must consist solely of lowercase " + "letters, numbers, hyphens and underscores.")) + (validate-regex group-id #"^[a-z0-9_.-]+$" + (str "Group names must consist solely of lowercase " + "letters, numbers, hyphens and underscores.")) + ;; Maven's pretty accepting of version numbers, but so far in 2.5 years + ;; bar one broken non-ascii exception only these characters have been used. + ;; Even if we manage to support obscure characters some filesystems do not + ;; and some tools fail to escape URLs properly. So to keep things nice and + ;; compatible for everyone let's lock it down. + (validate-regex version #"^[a-zA-Z0-9_.+-]+$" + (str "Version strings must consist solely of letters, " + "numbers, dots, pluses, hyphens and underscores.")) + (when (.exists (io/file (config :repo) group-id artifact-id version)) + (throw (ex-info "Redeploying is not allowed." {:status 403})))) + (defn event-log-file [type] (io/file (config :event-dir) (str (name type) ".clj"))) @@ -18,6 +45,7 @@ (spit filename content :append true)))) (defn record-deploy [{:keys [group-id artifact-id version]} deployed-by file] + (validate-deploy group-id artifact-id version) (when (.endsWith (str file) ".pom") (with-open [index (clucy/disk-index (config :index-path))] (search/index-pom index file))) @@ -89,4 +117,4 @@ (io/file (config :repo) group_name jar_name version (format "%s-%s.pom" jar_name version))) (catch Exception e - (println (.getMessage e)))))))) \ No newline at end of file + (println (.getMessage e)))))))) diff --git a/src/clojars/routes/repo.clj b/src/clojars/routes/repo.clj index 35034e05..3d966b04 100644 --- a/src/clojars/routes/repo.clj +++ b/src/clojars/routes/repo.clj @@ -71,6 +71,11 @@ ;;TODO once db is removed this whole if block ;;can be reduced to the common ;;(try (save-to-file...) (catch ...)) + ;; Be consistent with scp only recording pom or jar + (when (some #(.endsWith filename %) [".pom" ".jar"]) + (ev/record-deploy {:group-id groupname + :artifact-id artifact + :version version} account file)) (if (.endsWith filename ".pom") (let [contents (slurp body) pom-info (merge (maven/pom-to-map @@ -87,12 +92,7 @@ (save-to-file file body) (catch java.io.IOException e (.delete file) - (throw e))))) - ;; Be consistent with scp only recording pom or jar - (when (some #(.endsWith filename %) [".pom" ".jar"]) - (ev/record-deploy {:group groupname - :artifact-id artifact - :version version} account file))) + (throw e)))))) {:status 201 :headers {} :body nil} (catch Exception e (pst e) diff --git a/src/clojars/scp.clj b/src/clojars/scp.clj index 0e274227..4fe51411 100644 --- a/src/clojars/scp.clj +++ b/src/clojars/scp.clj @@ -4,6 +4,7 @@ com.martiansoftware.nailgun.NGContext) (:require [clojure.set :as set] [clojure.java.io :as io] + [clojure.java.shell] [clojars.config :refer [config]] [clojars.maven :as maven] [clojars.db :as db] @@ -99,26 +100,27 @@ :when (not= (:name metafile) "maven-metadata.xml") :let [jarmap (maven/pom-to-map (:file metafile)) names (jar-names jarmap)]] - (if-let [jarfile (some jarfiles names)] - (do - (.println (.err ctx) (str "\nDeploying " (:group jarmap) "/" - (:name jarmap) " " (:version jarmap))) - (db/add-jar account jarmap true) - (aether/deploy :coordinates [(keyword (:group jarmap) - (:name jarmap)) - (:version jarmap)] - :jar-file jarfile - :pom-file (:file metafile) - :repository {"local" (file-repo (:repo config))} - :transfer-listener - (bound-fn [e] (@#'aether/default-listener-fn e))) - (db/add-jar account jarmap) - ;; TODO: doseq over files here - (ev/record-deploy (set/rename-keys jarmap {:name :artifact-id}) - account jarfile) - (ev/record-deploy (set/rename-keys jarmap {:name :artifact-id}) - account (:file metafile))) - (throw (Exception. (str "You need to give me one of: " names))))) + (let [jarfile (some jarfiles names)] + (when-not jarfile + (throw (Exception. (str "You need to give me one of: " names)))) + (.println (.err ctx) (str "\nDeploying " (:group jarmap) "/" + (:name jarmap) " " (:version jarmap))) + ;; TODO: doseq over files here + (ev/record-deploy (set/rename-keys jarmap {:name :artifact-id + :group :group-id}) + account jarfile) + (ev/record-deploy (set/rename-keys jarmap {:name :artifact-id + :group :group-id}) + account (:file metafile)) + (db/add-jar account jarmap) + (aether/deploy :coordinates [(keyword (:group jarmap) + (:name jarmap)) + (:version jarmap)] + :jar-file jarfile + :pom-file (:file metafile) + :repository {"local" (file-repo (:repo config))} + :transfer-listener + (bound-fn [e] (@#'aether/default-listener-fn e))))) (.println (.err ctx) (str "\nSuccess! Your jars are now available from " "http://clojars.org/")) (.flush (.err ctx)))) diff --git a/test/clojars/test/integration/scp.clj b/test/clojars/test/integration/scp.clj index 18b75e65..b69d5fba 100644 --- a/test/clojars/test/integration/scp.clj +++ b/test/clojars/test/integration/scp.clj @@ -1,5 +1,5 @@ (ns clojars.test.integration.scp - (:require [clojure.test :refer :all] + (:require [clojure.test :refer :all] [kerodon.core :refer :all] [kerodon.test :refer :all] [clojars.test.integration.steps :refer :all] @@ -15,7 +15,8 @@ (deftest user-can-register-and-scp (-> (session web/clojars-app) (register-as "dantheman" "test@example.org" "password" valid-ssh-key)) - (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n" + (is (= (str "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\n" + "Success! Your jars are now available from http://clojars.org/\n") (scp valid-ssh-key "test.jar" "test-0.0.1/test.pom"))) (-> (session web/clojars-app) (visit "/groups/fake") @@ -79,8 +80,8 @@ (is (thrown? Exception (scp (str valid-ssh-key "1") "test.jar" "test-0.0.1/test.pom"))) (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n" (scp (str valid-ssh-key "3") "test.jar" "test-0.0.1/test.pom"))) - (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n" - (scp (str valid-ssh-key "4") "test.jar" "test-0.0.1/test.pom"))))) + (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.2\n\nSuccess! Your jars are now available from http://clojars.org/\n" + (scp (str valid-ssh-key "4") "test.jar" "test-0.0.2/test.pom"))))) (deftest scp-wants-filenames-in-specific-format (-> (session web/clojars-app) @@ -95,5 +96,5 @@ (let [ssh-key (str valid-ssh-key "1")] (-> (session web/clojars-app) (register-as "fixture" "fixture@example.org" "password" ssh-key)) - (is (= "Welcome to Clojars, fixture!\n\nDeploying fake/test 0.0.1\nError: You don't have access to the fake group.\n" - (scp ssh-key "test.jar" "test-0.0.1/test.pom"))))) \ No newline at end of file + (is (= "Welcome to Clojars, fixture!\n\nDeploying fake/test 0.0.2\nError: You don't have access to the fake group.\n" + (scp ssh-key "test.jar" "test-0.0.2/test.pom"))))) diff --git a/test/clojars/test/integration/steps.clj b/test/clojars/test/integration/steps.clj index 2b2af662..4c35bf32 100644 --- a/test/clojars/test/integration/steps.clj +++ b/test/clojars/test/integration/steps.clj @@ -57,4 +57,4 @@ (with-out-str (scp/nail shim)) (catch Exception e)) - (.toString bytes))) \ No newline at end of file + (.toString bytes))) diff --git a/test/clojars/test/unit/db.clj b/test/clojars/test/unit/db.clj index 7fba5c41..1abc02f1 100644 --- a/test/clojars/test/unit/db.clj +++ b/test/clojars/test/unit/db.clj @@ -208,23 +208,6 @@ jars)) (is (= 3 (count jars)))))) -(deftest add-jar-validates-jar-name-format - (let [jarmap {:group "group-name" :version "1"}] - (is (thrown? Exception (db/add-jar "test-user" jarmap))) - (is (thrown? Exception (db/add-jar "test-user" - (assoc jarmap :name "HI")))) - (is (thrown? Exception (db/add-jar "test-user" - (assoc jarmap :name "lein*")))) - (is (thrown? Exception (db/add-jar "test-user" - (assoc jarmap :name "lein=")))) - (is (thrown? Exception (db/add-jar "test-user" - (assoc jarmap :name "lein>")))) - (is (thrown? Exception (db/add-jar "test-user" - (assoc jarmap :name "べ")))) - (is (db/add-jar "test-user" (assoc jarmap :name "hi"))) - (is (db/add-jar "test-user" (assoc jarmap :name "hi-"))) - (is (db/add-jar "test-user" (assoc jarmap :name "hi_1...2"))))) - (deftest add-jar-validates-group-name-format (let [jarmap {:name "jar-name" :version "1"}] (is (thrown? Exception (db/add-jar "test-user" jarmap)))