From d6c2d8eb1931454b412b0ac54ed5f17fd789b42d Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Wed, 8 Mar 2023 19:55:49 +0100
Subject: [PATCH 01/10] reftest: add an additional test case for W37, opam file
 with and without url section

It highlights that when no url is present, W37 is not triggered
---
 master_changes.md        |  1 +
 tests/reftests/lint.test | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/master_changes.md b/master_changes.md
index 7f9bf0d5d0f..fdcf877ed1a 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -178,6 +178,7 @@ users)
   * Add a test showing the behaviour of `opam switch list-available` [#6098 @kit-ty-kate]
   * Add a test for git packages with submodules [#6132 @kit-ty-kate]
   * Add basic test for `install --check` [#6122 @rjbou]
+  * lint: add an additional test case for W37 [#5561 @rjbou]
 
 ### Engine
   * Add a test filtering mechanism [#6105 @Keryan-dev]
diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test
index f9c47afbb2f..e2a0154967b 100644
--- a/tests/reftests/lint.test
+++ b/tests/reftests/lint.test
@@ -356,6 +356,18 @@ url { src:"https://u.rl" }
 ### opam lint ./lint.opam
 ${BASEDIR}/lint.opam: Warnings.
            warning 37: Missing field 'dev-repo'
+### <lint.opam>
+opam-version: "2.0"
+synopsis: "A word"
+description: "Two words."
+authors: "the testing team"
+homepage: "egapemoh"
+maintainer: "maint@tain.er"
+license: "ISC"
+bug-reports: "https://nobug"
+### # no url doesn't trigger the lint warning
+### opam lint ./lint.opam
+${BASEDIR}/lint.opam: Passed.
 ### : E39: Command 'make' called directly, use the built-in variable instead
 ### <lint.opam>
 opam-version: "2.0"

From 036776c045f991a9b113bb50542f7545ca854a9b Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Thu, 9 Mar 2023 14:55:01 +0100
Subject: [PATCH 02/10] reftest: lint, update E65 to test other urls scheme

---
 master_changes.md        |  7 +++++++
 tests/reftests/lint.test | 25 ++++++++++++++++++++++---
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/master_changes.md b/master_changes.md
index fdcf877ed1a..1ddbba63902 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -179,6 +179,13 @@ users)
   * Add a test for git packages with submodules [#6132 @kit-ty-kate]
   * Add basic test for `install --check` [#6122 @rjbou]
   * lint: add an additional test case for W37 [#5561 @rjbou]
+  * lint: update W37 to test other urls scheme [#5561 @rjbou]
+  * lint: update W37 to test other url schemes [#5561 @rjbou]
+  * lint: add E70 test [#5561 @rjbou]
+  * lint: add E71 test [#5561 @rjbou]
+  * lint: add E72 test [#5561 @rjbou]
+  * lint: add E73 test [#5561 @rjbou]
+  * lint: add more test cases for E59: special cases (conf, git url), with and without option `--with-check-upstream` [#5561 @rjbou]
 
 ### Engine
   * Add a test filtering mechanism [#6105 @Keryan-dev]
diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test
index e2a0154967b..30ac9dc3000 100644
--- a/tests/reftests/lint.test
+++ b/tests/reftests/lint.test
@@ -927,12 +927,31 @@ authors: "the testing team"
 homepage: "egapemoh"
 maintainer: "maint@tain.er"
 license: "ISC"
-dev-repo: "hg+https://to@li.nt"
+dev-repo: "git+file://./../to@li.nt"
 bug-reports: "https://nobug"
-url { src:"file://./my/path" }
+url {
+  src:"file://./my/url/path"
+  checksum: "md5=00000000000000000000000000000000"
+  mirrors: [
+    "file:///good/mirror"
+    "file:///wrong/../mirror"
+    "file:///another/./mirror/"
+  ]
+}
+extra-source "relative" {
+  src:"file:///my/./../extrasource/path"
+  checksum: "md5=00000000000000000000000000000000"
+}
+extra-source "notrelative" {
+  src:"file:///my/extrasource..path.patch"
+  checksum: "md5=00000000000000000000000000000000"
+}
+pin-depends: [
+  [ "pinned.1" "file:///my/../pinned/package" ]
+]
 ### opam lint ./lint.opam
 ${BASEDIR}/lint.opam: Errors.
-             error 65: URLs must be absolute: "./my/path"
+             error 65: URLs must be absolute: "./my/url/path", "/wrong/../mirror", "./../to@li.nt", "/my/./../extrasource/path", "/my/extrasource..path.patch", "/my/../pinned/package"
 # Return code 1 #
 ### : W66: String that can't be resolved to bool in filtered package formula
 ### <lint.opam>

From fcd6f527c9b9e54dfb638b8865265ccb6f142b37 Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Wed, 8 Mar 2023 18:45:01 +0100
Subject: [PATCH 03/10] lint: add E70 for extra-files duplication

---
 master_changes.md             |  2 +
 src/state/opamFileTools.ml    | 31 ++++++++++++++++
 tests/reftests/extrafile.test | 16 ++++++--
 tests/reftests/lint.test      | 70 +++++++++++++++++++++++++++++++++++
 4 files changed, 115 insertions(+), 4 deletions(-)

diff --git a/master_changes.md b/master_changes.md
index 1ddbba63902..9a151c4b4d3 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -69,6 +69,7 @@ users)
 ## Source
 
 ## Lint
+  * Add E70 to check `extra-files:` duplicated fields [#5561 @rjbou]
 
 ## Repository
  * Mitigate curl/curl#13845 by falling back from --write-out to --fail if exit code 43 is returned by curl [#6168 @dra27 - fix #6120]
@@ -186,6 +187,7 @@ users)
   * lint: add E72 test [#5561 @rjbou]
   * lint: add E73 test [#5561 @rjbou]
   * lint: add more test cases for E59: special cases (conf, git url), with and without option `--with-check-upstream` [#5561 @rjbou]
+  * lint: add E70 test [#5561 @rjbou]
 
 ### Engine
   * Add a test filtering mechanism [#6105 @Keryan-dev]
diff --git a/src/state/opamFileTools.ml b/src/state/opamFileTools.ml
index 75f25bc6d4d..e1930c49ddd 100644
--- a/src/state/opamFileTools.ml
+++ b/src/state/opamFileTools.ml
@@ -401,6 +401,27 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t =
     not (OpamFile.OPAM.has_flag Pkgflag_Conf t) &&
     url_vcs = Some false
   in
+  let check_double compare to_str lst =
+    let double =
+      List.sort compare lst
+      |> List.fold_left (fun (last, dbl) elem ->
+          match last with
+          | Some last ->
+            if compare last elem = 0 then
+              Some elem, OpamStd.String.Map.update (to_str elem) ((+) 1) 1 dbl
+            else
+              Some elem, dbl
+          | None -> Some elem, dbl)
+        (None, OpamStd.String.Map.empty)
+      |> snd
+    in
+    if OpamStd.String.Map.is_empty double then false, None else
+      true,
+      Some (List.map (fun (elem, occ) ->
+          Printf.sprintf "%s: %d occurence%s"
+            elem occ (if occ = 1 then "" else "s"))
+          (OpamStd.String.Map.bindings double))
+  in
   let warnings = [
     cond 20 `Warning
       "Field 'opam-version' refers to the patch version of opam, it \
@@ -1011,6 +1032,16 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t =
              p v p v)
            vars)
        (vars <> []));
+    (let has_double, detail =
+       check_double OpamFilename.Base.compare OpamFilename.Base.to_string
+         (match OpamFile.OPAM.extra_files t with
+          | Some extra_files -> List.map fst extra_files
+          | None -> [])
+     in
+     cond 70 `Error
+       "Field 'extra-files' contains duplicated files"
+       ?detail
+       has_double);
   ]
   in
   format_errors @
diff --git a/tests/reftests/extrafile.test b/tests/reftests/extrafile.test
index bb30f20a6a4..8ab8e312d66 100644
--- a/tests/reftests/extrafile.test
+++ b/tests/reftests/extrafile.test
@@ -200,9 +200,13 @@ Successfully extracted to ${BASEDIR}/good-md5.1
 Clearing cache of downloaded files
 ### :I:2: good md5 & sha256
 ### opam lint --package good-md5-good-sha256
-<default>/good-md5-good-sha256.1: Passed.
+<default>/good-md5-good-sha256.1: Errors.
+             error 70: Field 'extra-files' contains duplicated files: "p.patch: 2 occurences"
+# Return code 1 #
 ### opam lint --package good-md5-good-sha256 --check-upstream
-<default>/good-md5-good-sha256.1: Passed.
+<default>/good-md5-good-sha256.1: Errors.
+             error 70: Field 'extra-files' contains duplicated files: "p.patch: 2 occurences"
+# Return code 1 #
 ### opam install good-md5-good-sha256
 The following actions will be performed:
 === install 1 package
@@ -284,9 +288,13 @@ Bad hash for   - ${BASEDIR}/OPAM/repo/default/packages/bad-md5/bad-md5.1/files/p
 Clearing cache of downloaded files
 ### :I:4: good md5 & bad sha256
 ### opam lint --package good-md5-bad-sha256
-<default>/good-md5-bad-sha256.1: Passed.
+<default>/good-md5-bad-sha256.1: Errors.
+             error 70: Field 'extra-files' contains duplicated files: "p.patch: 2 occurences"
+# Return code 1 #
 ### opam lint --package good-md5-bad-sha256 --check-upstream
-<default>/good-md5-bad-sha256.1: Passed.
+<default>/good-md5-bad-sha256.1: Errors.
+             error 70: Field 'extra-files' contains duplicated files: "p.patch: 2 occurences"
+# Return code 1 #
 ### opam install good-md5-bad-sha256
 The following actions will be performed:
 === install 1 package
diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test
index 30ac9dc3000..94c4d276b71 100644
--- a/tests/reftests/lint.test
+++ b/tests/reftests/lint.test
@@ -710,6 +710,7 @@ echo "extra-files: [ \"more-file-good-md5\" \"md5=$hsh\" ]" >> REPO/packages/lin
 # Return code 1 #
 ### opam lint --package lint.2
 <default>/lint.2: Passed.
+### OPAMREPOSITORYTARRING=0
 ### : W54: External dependencies should not contain spaces nor empty string
 ### <lint.opam>
 opam-version: "2.0"
@@ -1020,3 +1021,72 @@ messages: [
 ### opam lint ./lint.opam
 ${BASEDIR}/lint.opam: Warnings.
            warning 69: Package name in variable in string interpolation contains several '+', use: "'?conf-g++:installed:' instead of 'conf-g++:installed'"
+### : E70: Field 'extra-files' contains duplicated files
+### <pkg:lint.3>
+opam-version: "2.0"
+synopsis: "A word"
+description: "Two words."
+authors: "the testing team"
+homepage: "egapemoh"
+maintainer: "maint@tain.er"
+license: "ISC"
+dev-repo: "hg+https://to@li.nt"
+bug-reports: "https://nobug"
+### <pkg:lint.3:single-file>
+file
+### <pkg:lint.3:double-file>
+file
+### <pkg:lint.3:quadra-file>
+file
+### <hash.sh>
+set -ue
+path=REPO/packages/lint/lint.3
+md5=`openssl md5 "$path/files/single-file" | cut -f2 -d' '`
+cat << EOF >> "$path/opam"
+extra-files:[
+  ["single-file" "md5=$md5"]
+  ["double-file" "md5=$md5"]
+  ["double-file" "md5=$md5"]
+  ["quadra-file" "md5=$md5"]
+  ["quadra-file" "md5=$md5"]
+  ["quadra-file" "md5=$md5"]
+  ["quadra-file" "md5=$md5"]
+]
+EOF
+### sh hash.sh
+### opam update
+
+<><> Updating package repositories ><><><><><><><><><><><><><><><><><><><><><><>
+[default] synchronised from file://${BASEDIR}/REPO
+Now run 'opam upgrade' to apply any package updates.
+### opam lint --package lint.3
+<default>/lint.3: Errors.
+             error 70: Field 'extra-files' contains duplicated files: "double-file: 2 occurences", "quadra-file: 4 occurences"
+# Return code 1 #
+### : several extra-source
+### <lint.opam>
+opam-version: "2.0"
+synopsis: "A word"
+description: "Two words."
+authors: "the testing team"
+homepage: "egapemoh"
+maintainer: "maint@tain.er"
+license: "ISC"
+dev-repo: "hg+https://to@li.nt"
+bug-reports: "https://nobug"
+extra-source "double-file" {
+  src:"https://git.url/repo.tgz"
+  checksum:"md5=00000000000000000000000000000000"
+}
+extra-source "single-file" {
+  src:"https://git.url/repo.tgz"
+  checksum:"md5=00000000000000000000000000000000"
+}
+extra-source "double-file" {
+  src:"https://git.url/repo.tgz"
+  checksum:"md5=00000000000000000000000000000000"
+}
+### opam lint ./lint.opam
+${BASEDIR}/lint.opam: Errors.
+             error  3: File format error in 'extra-source': Duplicate section extra-source
+# Return code 1 #

From 5a8b1be6820520efa8a09c0c6f7c2f7409b32fb1 Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Wed, 8 Mar 2023 19:38:50 +0100
Subject: [PATCH 04/10] hash: export compare kind

---
 master_changes.md     | 1 +
 src/core/opamHash.mli | 1 +
 2 files changed, 2 insertions(+)

diff --git a/master_changes.md b/master_changes.md
index 9a151c4b4d3..c226d442990 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -248,3 +248,4 @@ users)
 
 ## opam-core
   * `OpamStd.Env`: add `env_string_list` for parsing string list environment variables (comma separated) [#5682 @desumn]
+  * `OpamHash`: export `compare_kind` [#5561 @rjbou]
diff --git a/src/core/opamHash.mli b/src/core/opamHash.mli
index deb5c7ff959..e4653537a8f 100644
--- a/src/core/opamHash.mli
+++ b/src/core/opamHash.mli
@@ -27,6 +27,7 @@ val sha512: string -> t
 include OpamStd.ABSTRACT with type t := t
 
 val of_string_opt: string -> t option
+val compare_kind: kind -> kind -> int
 
 (** Check if [hash] contains only 0 *)
 val is_null: t -> bool

From 35cd1481da930f9c21e7617b83463d4c7542e1bd Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Wed, 8 Mar 2023 19:40:22 +0100
Subject: [PATCH 05/10] lint: add E71 for checking if the same checksum
 algorithm is used several times for a given url in url section

---
 master_changes.md           |  2 ++
 src/state/opamFileTools.ml  | 10 ++++++++++
 tests/reftests/archive.test | 10 ++++++++--
 tests/reftests/lint.test    | 26 ++++++++++++++++++++++++++
 4 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/master_changes.md b/master_changes.md
index c226d442990..c80c2c3e1a4 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -70,6 +70,7 @@ users)
 
 ## Lint
   * Add E70 to check `extra-files:` duplicated fields [#5561 @rjbou]
+  * Add E71 to check if the same checksum algorithm is used several times for a given url in `url` section [#5561 @rjbou]
 
 ## Repository
  * Mitigate curl/curl#13845 by falling back from --write-out to --fail if exit code 43 is returned by curl [#6168 @dra27 - fix #6120]
@@ -188,6 +189,7 @@ users)
   * lint: add E73 test [#5561 @rjbou]
   * lint: add more test cases for E59: special cases (conf, git url), with and without option `--with-check-upstream` [#5561 @rjbou]
   * lint: add E70 test [#5561 @rjbou]
+  * lint: add E71 test [#5561 @rjbou]
 
 ### Engine
   * Add a test filtering mechanism [#6105 @Keryan-dev]
diff --git a/src/state/opamFileTools.ml b/src/state/opamFileTools.ml
index e1930c49ddd..b50145904cc 100644
--- a/src/state/opamFileTools.ml
+++ b/src/state/opamFileTools.ml
@@ -1042,6 +1042,16 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t =
        "Field 'extra-files' contains duplicated files"
        ?detail
        has_double);
+    (let has_double, detail =
+       check_double OpamHash.compare_kind OpamHash.string_of_kind
+         (match OpamFile.OPAM.url t with
+          | Some url ->
+            List.map OpamHash.kind (OpamFile.URL.checksum url)
+          | None -> [])
+     in
+     cond 71 `Error
+       "Field 'url.checksum' contains duplicated checksums"
+       ?detail has_double);
   ]
   in
   format_errors @
diff --git a/tests/reftests/archive.test b/tests/reftests/archive.test
index aa6fcc10680..60d43572c68 100644
--- a/tests/reftests/archive.test
+++ b/tests/reftests/archive.test
@@ -475,12 +475,15 @@ Successfully extracted to ${BASEDIR}/no-checksum.1
 Clearing cache of downloaded files
 ### :I:6: multiple md5
 ### opam lint --package multiple-md5
-<default>/multiple-md5.1: Passed.
+<default>/multiple-md5.1: Errors.
+             error 71: Field 'url.checksum' contains duplicated checksums: "md5: 2 occurences"
+# Return code 1 #
 ### opam lint --package multiple-md5 --check-upstream | '[0-9a-z]{32}' -> 'hash'
 <default>/multiple-md5.1: Errors.
              error 60: Upstream check failed: "The archive doesn't match checksum:
                 - archive: md5=hash, in opam file: md5=hash
               ."
+             error 71: Field 'url.checksum' contains duplicated checksums: "md5: 2 occurences"
 # Return code 1 #
 ### opam install multiple-md5 | '[0-9a-z]{32}' -> 'hash'
 The following actions will be performed:
@@ -781,7 +784,9 @@ OpamSolution.Fetch_fail("Checksum mismatch")
 Clearing cache of downloaded files
 ### :I:10: clash with all md5
 ### opam lint --package clash-with-all-md5s
-<default>/clash-with-all-md5s.666: Passed.
+<default>/clash-with-all-md5s.666: Errors.
+             error 71: Field 'url.checksum' contains duplicated checksums: "md5: 17 occurences"
+# Return code 1 #
 ### opam lint --package clash-with-all-md5s --check-upstream | '[0-9a-z]{32,64}' -> 'hash'
 <default>/clash-with-all-md5s.666: Errors.
              error 60: Upstream check failed: "The archive doesn't match checksums:
@@ -803,6 +808,7 @@ Clearing cache of downloaded files
                 - archive: md5=hash, in opam file: md5=hash
                 - archive: sha256=hash, in opam file: sha256=hash
               ."
+             error 71: Field 'url.checksum' contains duplicated checksums: "md5: 17 occurences"
 # Return code 1 #
 ### opam install clash-with-all-md5s | '[0-9a-z]{32}' -> 'hash'
 The following actions will be performed:
diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test
index 94c4d276b71..7e3c6b70e9d 100644
--- a/tests/reftests/lint.test
+++ b/tests/reftests/lint.test
@@ -1090,3 +1090,29 @@ extra-source "double-file" {
 ${BASEDIR}/lint.opam: Errors.
              error  3: File format error in 'extra-source': Duplicate section extra-source
 # Return code 1 #
+### : E71: Field 'url.checksum' contains duplicated checksums
+### <lint.opam>
+opam-version: "2.0"
+synopsis: "A word"
+description: "Two words."
+authors: "the testing team"
+homepage: "egapemoh"
+maintainer: "maint@tain.er"
+license: "ISC"
+dev-repo: "hg+https://to@li.nt"
+bug-reports: "https://nobug"
+url {
+  src:"an-archive.tgz"
+  checksum:[
+  "md5=00000000000000000000000000000000"
+  "md5=11111111111111111111111111111111"
+  "sha256=2222222222222222222222222222222222222222222222222222222222222222"
+  "sha256=3333333333333333333333333333333333333333333333333333333333333333"
+  "sha256=4444444444444444444444444444444444444444444444444444444444444444"
+  "sha512=55555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555"
+  ]
+}
+### opam lint ./lint.opam
+${BASEDIR}/lint.opam: Errors.
+             error 71: Field 'url.checksum' contains duplicated checksums: "md5: 2 occurences", "sha256: 3 occurences"
+# Return code 1 #

From ab348e21107b11b9ceda40c91545dc3db88fa84c Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Wed, 14 Aug 2024 16:49:29 +0200
Subject: [PATCH 06/10] lint: add E72 for checking if the same checksum
 algorithm is used several times for a given url in extra-sources section

---
 master_changes.md               |  2 ++
 src/state/opamFileTools.ml      | 26 +++++++++++++++++++++
 tests/reftests/extrasource.test | 16 +++++++++----
 tests/reftests/lint.test        | 41 +++++++++++++++++++++++++++++++++
 4 files changed, 81 insertions(+), 4 deletions(-)

diff --git a/master_changes.md b/master_changes.md
index c80c2c3e1a4..9eb8ce62ece 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -71,6 +71,7 @@ users)
 ## Lint
   * Add E70 to check `extra-files:` duplicated fields [#5561 @rjbou]
   * Add E71 to check if the same checksum algorithm is used several times for a given url in `url` section [#5561 @rjbou]
+  * Add E72 to check if the same checksum algorithm is used several times for a given url in `extra-sources` section [#5561 @rjbou]
 
 ## Repository
  * Mitigate curl/curl#13845 by falling back from --write-out to --fail if exit code 43 is returned by curl [#6168 @dra27 - fix #6120]
@@ -190,6 +191,7 @@ users)
   * lint: add more test cases for E59: special cases (conf, git url), with and without option `--with-check-upstream` [#5561 @rjbou]
   * lint: add E70 test [#5561 @rjbou]
   * lint: add E71 test [#5561 @rjbou]
+  * lint: add E72 test [#5561 @rjbou]
 
 ### Engine
   * Add a test filtering mechanism [#6105 @Keryan-dev]
diff --git a/src/state/opamFileTools.ml b/src/state/opamFileTools.ml
index b50145904cc..a860e147952 100644
--- a/src/state/opamFileTools.ml
+++ b/src/state/opamFileTools.ml
@@ -1052,6 +1052,32 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t =
      cond 71 `Error
        "Field 'url.checksum' contains duplicated checksums"
        ?detail has_double);
+    (let has_double, detail =
+       OpamFile.OPAM.extra_sources t
+       |> List.rev_map (fun (basename, url) ->
+           basename,
+           OpamFile.URL.checksum url
+           |> List.rev_map OpamHash.kind
+           |> check_double
+             OpamHash.compare_kind
+             OpamHash.string_of_kind)
+       |> List.fold_left (fun (has_double, details) (basename, (double, detail)) ->
+           let has_double = has_double || double in
+           let details =
+             match detail with
+             | None -> details
+             | Some detail ->
+               Printf.sprintf "%s have %s"
+                 (OpamFilename.Base.to_string basename)
+                 (OpamStd.Format.pretty_list detail)
+               :: details
+           in
+           has_double, details) (false, [])
+       |> (function hd, [] -> hd, None | hd, d -> hd, Some d)
+     in
+     cond 72 `Error
+       "Field 'extra-sources' contains duplicated checksums"
+       ?detail has_double);
   ]
   in
   format_errors @
diff --git a/tests/reftests/extrasource.test b/tests/reftests/extrasource.test
index fa65a343d65..c5445355894 100644
--- a/tests/reftests/extrasource.test
+++ b/tests/reftests/extrasource.test
@@ -549,9 +549,13 @@ Successfully extracted to ${BASEDIR}/no-checksum.1
 Clearing cache of downloaded files
 ### :I:6: multiple md5
 ### opam lint --package multiple-md5
-<default>/multiple-md5.1: Passed.
+<default>/multiple-md5.1: Errors.
+             error 72: Field 'extra-sources' contains duplicated checksums: "i-am-a-patch have md5: 2 occurences"
+# Return code 1 #
 ### opam lint --package multiple-md5 --check-upstream
-<default>/multiple-md5.1: Passed.
+<default>/multiple-md5.1: Errors.
+             error 72: Field 'extra-sources' contains duplicated checksums: "i-am-a-patch have md5: 2 occurences"
+# Return code 1 #
 ### opam install multiple-md5 | '[0-9a-z]{32}' -> 'hash'
 The following actions will be performed:
 === install 1 package
@@ -839,9 +843,13 @@ OpamSolution.Fetch_fail("Checksum mismatch")
 Clearing cache of downloaded files
 ### :I:10: clash with all md5
 ### opam lint --package clash-with-all-md5s
-<default>/clash-with-all-md5s.666: Passed.
+<default>/clash-with-all-md5s.666: Errors.
+             error 72: Field 'extra-sources' contains duplicated checksums: "i-am-a-patch have md5: 17 occurences"
+# Return code 1 #
 ### opam lint --package clash-with-all-md5s --check-upstream
-<default>/clash-with-all-md5s.666: Passed.
+<default>/clash-with-all-md5s.666: Errors.
+             error 72: Field 'extra-sources' contains duplicated checksums: "i-am-a-patch have md5: 17 occurences"
+# Return code 1 #
 ### opam install clash-with-all-md5s | '[0-9a-z]{32}' -> 'hash'
 The following actions will be performed:
 === install 1 package
diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test
index 7e3c6b70e9d..54fa33e1851 100644
--- a/tests/reftests/lint.test
+++ b/tests/reftests/lint.test
@@ -1116,3 +1116,44 @@ url {
 ${BASEDIR}/lint.opam: Errors.
              error 71: Field 'url.checksum' contains duplicated checksums: "md5: 2 occurences", "sha256: 3 occurences"
 # Return code 1 #
+### : E72: Field 'extra-sources' contains duplicated checksums
+### <lint.opam>
+opam-version: "2.0"
+synopsis: "A word"
+description: "Two words."
+authors: "the testing team"
+homepage: "egapemoh"
+maintainer: "maint@tain.er"
+license: "ISC"
+dev-repo: "hg+https://to@li.nt"
+bug-reports: "https://nobug"
+extra-source "double-file" {
+  src:"https://git.url/repo.tgz"
+  checksum:[
+  "md5=00000000000000000000000000000000"
+  "md5=11111111111111111111111111111111"
+  "sha256=2222222222222222222222222222222222222222222222222222222222222222"
+  "sha256=3333333333333333333333333333333333333333333333333333333333333333"
+  "sha256=4444444444444444444444444444444444444444444444444444444444444444"
+  "sha512=55555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555"
+  ]
+}
+extra-source "double-file2" {
+  src:"https://git.url/repo.tgz"
+  checksum:[
+  "md5=00000000000000000000000000000000"
+  "md5=11111111111111111111111111111111"
+  ]
+}
+extra-source "double-file3" {
+  src:"https://git.url/repo.tgz"
+  checksum:[
+  "md5=00000000000000000000000000000000"
+  "sha256=3333333333333333333333333333333333333333333333333333333333333333"
+  "sha512=55555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555555"
+  ]
+}
+### opam lint ./lint.opam
+${BASEDIR}/lint.opam: Errors.
+             error 72: Field 'extra-sources' contains duplicated checksums: "double-file have md5: 2 occurences and sha256: 3 occurences", "double-file2 have md5: 2 occurences"
+# Return code 1 #

From 17f2447b8ceb2000304c3178d62e3690bab8d580 Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Fri, 24 Mar 2023 15:36:49 +0100
Subject: [PATCH 07/10] filename: add might_escape function

---
 master_changes.md         |  1 +
 src/core/opamFilename.ml  | 10 ++++++++++
 src/core/opamFilename.mli |  3 +++
 3 files changed, 14 insertions(+)

diff --git a/master_changes.md b/master_changes.md
index 9eb8ce62ece..aa8ccd10782 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -253,3 +253,4 @@ users)
 ## opam-core
   * `OpamStd.Env`: add `env_string_list` for parsing string list environment variables (comma separated) [#5682 @desumn]
   * `OpamHash`: export `compare_kind` [#5561 @rjbou]
+  * `OpamFilename`: add `might_escape` to check if a path is escapable, ie contains `<sep>..<sep>` [#5561 @rjbou]
diff --git a/src/core/opamFilename.ml b/src/core/opamFilename.ml
index 27c5dbf59bf..e94fbbaa497 100644
--- a/src/core/opamFilename.ml
+++ b/src/core/opamFilename.ml
@@ -9,6 +9,16 @@
 (*                                                                        *)
 (**************************************************************************)
 
+let might_escape ~sep path =
+  let sep =
+    match sep with
+    | `Unix -> Re.char '/'
+    | `Windows -> Re.alt Re.[  char '\\'; char '/' ]
+    | `Unspecified -> Re.str Filename.dir_sep
+  in
+  List.exists (String.equal Filename.parent_dir_name)
+    Re.(split (compile sep) path)
+
 module Base = struct
   include OpamStd.AbstractString
 
diff --git a/src/core/opamFilename.mli b/src/core/opamFilename.mli
index 44b9e3a467c..f27f832360a 100644
--- a/src/core/opamFilename.mli
+++ b/src/core/opamFilename.mli
@@ -12,6 +12,9 @@
 (** Higher level file and directory name manipulation AND file operations,
     wrappers on OpamSystem using the filename type *)
 
+(* Returns [true] if string contains '..' between directory separators *)
+val might_escape: sep:[`Unix | `Windows | `Unspecified ] -> string -> bool
+
 (** Basenames *)
 module Base: sig
   include OpamStd.ABSTRACT

From b785fc46ed64b5b11b7b2bb0945317cb5fd81c42 Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Fri, 24 Mar 2023 12:56:32 +0100
Subject: [PATCH 08/10] lint: add E73 for relative path on extra-files

---
 master_changes.md             |  2 ++
 src/state/opamFileTools.ml    | 16 +++++++++++++++-
 tests/reftests/extrafile.test |  2 ++
 tests/reftests/lint.test      | 28 +++++++++++++++++++++++++++-
 4 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/master_changes.md b/master_changes.md
index aa8ccd10782..3aa506c09cc 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -72,6 +72,7 @@ users)
   * Add E70 to check `extra-files:` duplicated fields [#5561 @rjbou]
   * Add E71 to check if the same checksum algorithm is used several times for a given url in `url` section [#5561 @rjbou]
   * Add E72 to check if the same checksum algorithm is used several times for a given url in `extra-sources` section [#5561 @rjbou]
+  * Add E73 to check that paths in `extra-files:` are not escapable [#5561 @rjbou]
 
 ## Repository
  * Mitigate curl/curl#13845 by falling back from --write-out to --fail if exit code 43 is returned by curl [#6168 @dra27 - fix #6120]
@@ -192,6 +193,7 @@ users)
   * lint: add E70 test [#5561 @rjbou]
   * lint: add E71 test [#5561 @rjbou]
   * lint: add E72 test [#5561 @rjbou]
+  * lint: add E73 test [#5561 @rjbou]
 
 ### Engine
   * Add a test filtering mechanism [#6105 @Keryan-dev]
diff --git a/src/state/opamFileTools.ml b/src/state/opamFileTools.ml
index a860e147952..ced443f74a8 100644
--- a/src/state/opamFileTools.ml
+++ b/src/state/opamFileTools.ml
@@ -930,7 +930,7 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t =
               ("file" | "path" | "local" | "rsync") -> true
             | _, _ -> false)
            && (Filename.is_relative u.path
-               || OpamStd.String.contains ~sub:".." u.path))
+               || OpamFilename.might_escape ~sep:`Unix u.path))
          (all_urls t)
      in
      cond 65 `Error
@@ -1078,6 +1078,20 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t =
      cond 72 `Error
        "Field 'extra-sources' contains duplicated checksums"
        ?detail has_double);
+    (let relative =
+       match t.extra_files with
+       | None -> []
+       | Some extra_files ->
+         List.filter_map (fun (base, _) ->
+             let path = OpamFilename.Base.to_string base in
+             if OpamFilename.might_escape ~sep:`Unix path then
+               Some path else None)
+           extra_files
+     in
+     cond 73 `Error
+       "Field 'extra-files' contains path with '..'"
+       ~detail:relative
+       (relative <> []));
   ]
   in
   format_errors @
diff --git a/tests/reftests/extrafile.test b/tests/reftests/extrafile.test
index 8ab8e312d66..cff98501671 100644
--- a/tests/reftests/extrafile.test
+++ b/tests/reftests/extrafile.test
@@ -528,10 +528,12 @@ Clearing cache of downloaded files
 ### opam lint --package escape-good-md5
 <default>/escape-good-md5.1: Errors.
              error 53: Mismatching 'extra-files:' field: "../../../no-checksum/no-checksum.1/files/p.patch"
+             error 73: Field 'extra-files' contains path with '..': "../../../no-checksum/no-checksum.1/files/p.patch"
 # Return code 1 #
 ### opam lint --package escape-good-md5 --check-upstream
 <default>/escape-good-md5.1: Errors.
              error 53: Mismatching 'extra-files:' field: "../../../no-checksum/no-checksum.1/files/p.patch"
+             error 73: Field 'extra-files' contains path with '..': "../../../no-checksum/no-checksum.1/files/p.patch"
 # Return code 1 #
 ### # ERROR it copies it to build dir ^ relative path -> escape!!!!
 ### # currently writing in /tmp as a common it copies in
diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test
index 54fa33e1851..c8ac4ac4991 100644
--- a/tests/reftests/lint.test
+++ b/tests/reftests/lint.test
@@ -952,7 +952,7 @@ pin-depends: [
 ]
 ### opam lint ./lint.opam
 ${BASEDIR}/lint.opam: Errors.
-             error 65: URLs must be absolute: "./my/url/path", "/wrong/../mirror", "./../to@li.nt", "/my/./../extrasource/path", "/my/extrasource..path.patch", "/my/../pinned/package"
+             error 65: URLs must be absolute: "./my/url/path", "/wrong/../mirror", "./../to@li.nt", "/my/./../extrasource/path", "/my/../pinned/package"
 # Return code 1 #
 ### : W66: String that can't be resolved to bool in filtered package formula
 ### <lint.opam>
@@ -1157,3 +1157,29 @@ extra-source "double-file3" {
 ${BASEDIR}/lint.opam: Errors.
              error 72: Field 'extra-sources' contains duplicated checksums: "double-file have md5: 2 occurences and sha256: 3 occurences", "double-file2 have md5: 2 occurences"
 # Return code 1 #
+### : E73: Field 'extra-files' contains path with '..'
+### <lint.opam>
+opam-version: "2.0"
+synopsis: "A word"
+description: "Two words."
+authors: "the testing team"
+homepage: "egapemoh"
+maintainer: "maint@tain.er"
+license: "ISC"
+dev-repo: "hg+https://to@li.nt"
+bug-reports: "https://nobug"
+extra-source "double-file" {
+  src:"https://git.url/repo.tgz"
+  checksum:"md5=00000000000000000000000000000000"
+}
+extra-files: [
+  [ "./relative/path"            "md5=00000000000000000000000000000000"]
+  [ "/absolute/../relative/path" "md5=00000000000000000000000000000000"]
+  [ "/absolute/path"             "md5=00000000000000000000000000000000"]
+  [ "extra-files..patch.patch"   "md5=00000000000000000000000000000000"]
+]
+### opam lint ./lint.opam
+${BASEDIR}/lint.opam: Errors.
+             error 53: Mismatching 'extra-files:' field: "./relative/path", "/absolute/../relative/path", "/absolute/path", "extra-files..patch.patch"
+             error 73: Field 'extra-files' contains path with '..': "/absolute/../relative/path"
+# Return code 1 #

From dac75ebda3838eaa5a3cadbf8d1e98a6586621f5 Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Thu, 9 Mar 2023 15:05:58 +0100
Subject: [PATCH 09/10] reftest: add more test cases for W59

* Shows the difference of behaviour between 'opam lint' and 'opam lint --check-upstream'
* Add test cases for special cases: conf flag, url containing git url
---
 master_changes.md        |  2 ++
 tests/reftests/lint.test | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/master_changes.md b/master_changes.md
index 3aa506c09cc..348fba22764 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -194,6 +194,8 @@ users)
   * lint: add E71 test [#5561 @rjbou]
   * lint: add E72 test [#5561 @rjbou]
   * lint: add E73 test [#5561 @rjbou]
+  * lint: add more test cases for E59: special cases (conf, git url), with and without option `--with-check-upstream` [#5561 @rjbou]
+  * lint: add more test cases for W59: special cases (conf, git url), with and without `--with-check-upstream` [#5561 @rjbou]
 
 ### Engine
   * Add a test filtering mechanism [#6105 @Keryan-dev]
diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test
index c8ac4ac4991..35d2e56bd03 100644
--- a/tests/reftests/lint.test
+++ b/tests/reftests/lint.test
@@ -851,9 +851,48 @@ license: "ISC"
 dev-repo: "hg+https://to@li.nt"
 bug-reports: "https://nobug"
 url { src:"an-archive.tgz" }
+### opam lint ./lint.opam
+${BASEDIR}/lint.opam: Passed.
 ### opam lint ./lint.opam --check-upstream
 ${BASEDIR}/lint.opam: Warnings.
            warning 59: url doesn't contain a checksum
+### <lint.opam>
+opam-version: "2.0"
+synopsis: "A word"
+description: "Two words."
+authors: "the testing team"
+homepage: "egapemoh"
+maintainer: "maint@tain.er"
+license: "ISC"
+dev-repo: "hg+https://to@li.nt"
+bug-reports: "https://nobug"
+url { src:"an-archive.tgz" }
+flags: conf
+### # package with conf flag
+### opam lint ./lint.opam
+${BASEDIR}/lint.opam: Errors.
+             error 46: Package is flagged "conf" but has source, install or remove instructions
+# Return code 1 #
+### opam lint ./lint.opam --check-upstream
+${BASEDIR}/lint.opam: Errors.
+             error 46: Package is flagged "conf" but has source, install or remove instructions
+# Return code 1 #
+### <lint.opam>
+opam-version: "2.0"
+synopsis: "A word"
+description: "Two words."
+authors: "the testing team"
+homepage: "egapemoh"
+maintainer: "maint@tain.er"
+license: "ISC"
+dev-repo: "hg+https://to@li.nt"
+bug-reports: "https://nobug"
+url { src:"git+https://a/repo" }
+### # package with git url
+### opam lint ./lint.opam
+${BASEDIR}/lint.opam: Passed.
+### opam lint ./lint.opam --check-upstream
+${BASEDIR}/lint.opam: Passed.
 ### : E60: Upstream check failed
 ### <lint.opam>
 opam-version: "2.0"

From 0e4b791577196c19254ed001feb49b45552ce716 Mon Sep 17 00:00:00 2001
From: Raja Boujbel <raja.boujbel@ocamlpro.com>
Date: Wed, 8 Mar 2023 19:54:24 +0100
Subject: [PATCH 10/10] lint: update W59 (no checksum on url) to always display
 a warning, no more tied to --check-upstream option

---
 master_changes.md           |  1 +
 src/state/opamFileTools.ml  |  6 +++---
 tests/reftests/archive.test |  3 ++-
 tests/reftests/lint.test    |  8 ++++++--
 tests/reftests/show.test    | 27 +++++++++++++++------------
 5 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/master_changes.md b/master_changes.md
index 348fba22764..734531eb526 100644
--- a/master_changes.md
+++ b/master_changes.md
@@ -73,6 +73,7 @@ users)
   * Add E71 to check if the same checksum algorithm is used several times for a given url in `url` section [#5561 @rjbou]
   * Add E72 to check if the same checksum algorithm is used several times for a given url in `extra-sources` section [#5561 @rjbou]
   * Add E73 to check that paths in `extra-files:` are not escapable [#5561 @rjbou]
+  * Update W59 (no checksum in `url`) to always display a warning, untying it from `--check-upstream` [#5561 @rjbou]
 
 ## Repository
  * Mitigate curl/curl#13845 by falling back from --write-out to --fail if exit code 43 is returned by curl [#6168 @dra27 - fix #6120]
diff --git a/src/state/opamFileTools.ml b/src/state/opamFileTools.ml
index ced443f74a8..49e79e2f618 100644
--- a/src/state/opamFileTools.ml
+++ b/src/state/opamFileTools.ml
@@ -396,11 +396,11 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t =
         | #OpamUrl.version_control -> true
         | _ -> false)
   in
-  let check_upstream =
-    check_upstream &&
+  let is_url_archive =
     not (OpamFile.OPAM.has_flag Pkgflag_Conf t) &&
     url_vcs = Some false
   in
+  let check_upstream = check_upstream && is_url_archive in
   let check_double compare to_str lst =
     let double =
       List.sort compare lst
@@ -811,7 +811,7 @@ let t_lint ?check_extra_files ?(check_upstream=false) ?(all=false) t =
         Printf.sprintf "Found %s variable%s, predefined one%s" var s_ nvar)
        (rem_test || rem_doc));
     cond 59 `Warning "url doesn't contain a checksum"
-      (check_upstream &&
+      (is_url_archive &&
        OpamStd.Option.map OpamFile.URL.checksum t.url = Some []);
     (let upstream_error =
        if not check_upstream then None else
diff --git a/tests/reftests/archive.test b/tests/reftests/archive.test
index 60d43572c68..cea652b2ac6 100644
--- a/tests/reftests/archive.test
+++ b/tests/reftests/archive.test
@@ -430,7 +430,8 @@ Successfully extracted to ${BASEDIR}/good-sha256-good-md5.1
 Clearing cache of downloaded files
 ### :I:5: no checksum
 ### opam lint --package no-checksum
-<default>/no-checksum.1: Passed.
+<default>/no-checksum.1: Warnings.
+           warning 59: url doesn't contain a checksum
 ### opam lint --package no-checksum --check-upstream
 <default>/no-checksum.1: Warnings.
            warning 59: url doesn't contain a checksum
diff --git a/tests/reftests/lint.test b/tests/reftests/lint.test
index 35d2e56bd03..06e2ae6f48d 100644
--- a/tests/reftests/lint.test
+++ b/tests/reftests/lint.test
@@ -352,7 +352,10 @@ homepage: "egapemoh"
 maintainer: "maint@tain.er"
 license: "ISC"
 bug-reports: "https://nobug"
-url { src:"https://u.rl" }
+url {
+  src:"an-archive.tgz"
+  checksum: "md5=00000000000000000000000000000000"
+}
 ### opam lint ./lint.opam
 ${BASEDIR}/lint.opam: Warnings.
            warning 37: Missing field 'dev-repo'
@@ -852,7 +855,8 @@ dev-repo: "hg+https://to@li.nt"
 bug-reports: "https://nobug"
 url { src:"an-archive.tgz" }
 ### opam lint ./lint.opam
-${BASEDIR}/lint.opam: Passed.
+${BASEDIR}/lint.opam: Warnings.
+           warning 59: url doesn't contain a checksum
 ### opam lint ./lint.opam --check-upstream
 ${BASEDIR}/lint.opam: Warnings.
            warning 59: url doesn't contain a checksum
diff --git a/tests/reftests/show.test b/tests/reftests/show.test
index 7662c4741b6..13650c4ad7f 100644
--- a/tests/reftests/show.test
+++ b/tests/reftests/show.test
@@ -353,6 +353,7 @@ depexts devel/llvm10 llvm-10-dev
 opam-version: "2.0"
 url {
   src:"https://an.arch/i/ve.tgz"
+  checksum: "md5=00000000000000000000000000000000"
   mirrors:
     [ "https://a.mi/rror"
       "http://swhid.opam.ocaml.org/swh:1:dir:309cf2674ee7a0749978cf8265ab91a60aea0f7d" ]
@@ -364,18 +365,19 @@ name         swhid
 all-versions dev
 
 <><> Version-specific details <><><><><><><><><><><><><><><><><><><><><><><><><>
-version     dev
-pin         https://an.arch/i/ve.tgz
-url.src     "https://an.arch/i/ve.tgz"
-url.swhid   "swh:1:dir:309cf2674ee7a0749978cf8265ab91a60aea0f7d"
-homepage    "egapemoh"
-bug-reports "https://nobug"
-dev-repo    "hg+https://pkg@op.am"
-authors     "the testing team"
-maintainer  "maint@tain.er"
-license     "MIT"
-synopsis    A word
-description Two words.
+version      dev
+pin          https://an.arch/i/ve.tgz
+url.src      "https://an.arch/i/ve.tgz"
+url.swhid    "swh:1:dir:309cf2674ee7a0749978cf8265ab91a60aea0f7d"
+url.checksum "md5=00000000000000000000000000000000"
+homepage     "egapemoh"
+bug-reports  "https://nobug"
+dev-repo     "hg+https://pkg@op.am"
+authors      "the testing team"
+maintainer   "maint@tain.er"
+license      "MIT"
+synopsis     A word
+description  Two words.
 ### opam show ./swhid.opam --raw
 opam-version: "2.0"
 name: "swhid"
@@ -390,6 +392,7 @@ bug-reports: "https://nobug"
 dev-repo: "hg+https://pkg@op.am"
 url {
   src: "https://an.arch/i/ve.tgz"
+  checksum: "md5=00000000000000000000000000000000"
   mirrors: [
     "https://swhid.opam.ocaml.org/swh:1:dir:309cf2674ee7a0749978cf8265ab91a60aea0f7d"
     "https://a.mi/rror"