test: [#261]: do not allow uploading two torrents with the same canonical infohash

josecelano · josecelano · commit 110e1596d0af · 2023-09-12T11:43:38.000+01:00
If you upload a torrent, the infohash migth change if the `info`
dictionary contains custom fields. The Index removes non-standard custom
fields, and that generates a new infohash for the torrent.

If you upload a second torrent which is different from a previous one
only in the custom fields, the same canonical infohash will be
generated, so the torrent will be rejected as duplicated. The new
original infohash will be stored in the database.
diff --git a/.github/workflows/coverage.yaml b/.github/workflows/coverage.yaml
@@ -2,24 +2,55 @@ name: Coverage
 
 on:
   push:
-  pull_request:
+    branches:
+      - develop
+  pull_request_target:
+    branches:
+      - develop
 
 env:
   CARGO_TERM_COLOR: always
 
 jobs:
+  secrets:
+    name: Secrets
+    environment: coverage
+    runs-on: ubuntu-latest
+
+    outputs:
+      continue: ${{ steps.check.outputs.continue }}
+
+    steps:
+      - id: check
+        name: Check
+        env:
+          CODECOV_TOKEN: "${{ secrets.CODECOV_TOKEN }}"
+        if: "${{ env.CODECOV_TOKEN != '' }}"
+        run: echo "continue=true" >> $GITHUB_OUTPUT
+
   report:
     name: Report
+    environment: coverage
+    needs: secrets
+    if: needs.secrets.outputs.continue == 'true'
     runs-on: ubuntu-latest
     env:
       CARGO_INCREMENTAL: "0"
       RUSTFLAGS: "-Z profile -C codegen-units=1 -C inline-threshold=0 -C link-dead-code -C overflow-checks=off -C panic=abort -Z panic_abort_tests"
       RUSTDOCFLAGS: "-Z profile -C codegen-units=1 -C inline-threshold=0 -C link-dead-code -C overflow-checks=off -C panic=abort -Z panic_abort_tests"
 
     steps:
-      - id: checkout
-        name: Checkout Repository
-        uses: actions/checkout@v3
+      - id: checkout_push
+        if: github.event_name  == 'push'
+        name: Checkout Repository (Push)
+        uses: actions/checkout@v4
+
+      - id: checkout_pull_request_target
+        if: github.event_name  == 'pull_request_target'
+        name: Checkout Repository (Pull Request Target)
+        uses: actions/checkout@v4
+        with:
+          ref: "refs/pull/${{ github.event.pull_request.number }}/head"
 
       - id: setup
         name: Setup Toolchain
@@ -61,4 +92,4 @@ jobs:
           token: ${{ secrets.CODECOV_TOKEN }}
           files: ${{ steps.coverage.outputs.report }}
           verbose: true
-          fail_ci_if_error: true
+          fail_ci_if_error: true
diff --git a/.github/workflows/testing.yaml b/.github/workflows/testing.yaml
@@ -65,10 +65,14 @@ jobs:
         name: Run Lint Checks
         run: cargo clippy --tests --benches --examples --workspace --all-targets --all-features -- -D clippy::correctness -D clippy::suspicious -D clippy::complexity -D clippy::perf -D clippy::style -D clippy::pedantic
 
-      - id: doc
-        name: Run Documentation Checks
+      - id: testdoc
+        name: Run Documentation Tests
         run: cargo test --doc
 
+      - id: builddoc
+        name: Build Documentation
+        run: cargo doc --no-deps --bins --examples --workspace --all-features
+
   unit:
     name: Units
     runs-on: ubuntu-latest
diff --git a/src/databases/database.rs b/src/databases/database.rs
@@ -231,14 +231,17 @@ pub trait Database: Sync + Send {
         ))
     }
 
-    /// Returns the list of original infohashes ofr a canonical infohash.
+    /// Returns the list of all infohashes producing the same canonical infohash.
     ///
     /// When you upload a torrent the infohash migth change because the Index
     /// remove the non-standard fields in the `info` dictionary. That makes the
     /// infohash change. The canonical infohash is the resulting infohash.
     /// This function returns the original infohashes of a canonical infohash.
+    ///
+    /// If the original infohash was unknown, it returns the canonical infohash.
+    ///
     /// The relationship is 1 canonical infohash -> N original infohashes.
-    async fn get_torrent_original_info_hashes(&self, canonical: &InfoHash) -> Result<OriginalInfoHashes, Error>;
+    async fn get_torrent_canonical_info_hash_group(&self, canonical: &InfoHash) -> Result<OriginalInfoHashes, Error>;
 
     async fn insert_torrent_info_hash(&self, original: &InfoHash, canonical: &InfoHash) -> Result<(), Error>;
 
diff --git a/src/databases/mysql.rs b/src/databases/mysql.rs
@@ -592,7 +592,7 @@ impl Database for Mysql {
         }
     }
 
-    async fn get_torrent_original_info_hashes(&self, canonical: &InfoHash) -> Result<OriginalInfoHashes, database::Error> {
+    async fn get_torrent_canonical_info_hash_group(&self, canonical: &InfoHash) -> Result<OriginalInfoHashes, database::Error> {
         let db_info_hashes = query_as::<_, DbTorrentInfoHash>(
             "SELECT info_hash, canonical_info_hash, original_is_known FROM torrust_torrent_info_hashes WHERE canonical_info_hash = ?",
         )
diff --git a/src/databases/sqlite.rs b/src/databases/sqlite.rs
@@ -582,7 +582,7 @@ impl Database for Sqlite {
         }
     }
 
-    async fn get_torrent_original_info_hashes(&self, canonical: &InfoHash) -> Result<OriginalInfoHashes, database::Error> {
+    async fn get_torrent_canonical_info_hash_group(&self, canonical: &InfoHash) -> Result<OriginalInfoHashes, database::Error> {
         let db_info_hashes = query_as::<_, DbTorrentInfoHash>(
             "SELECT info_hash, canonical_info_hash, original_is_known FROM torrust_torrent_info_hashes WHERE canonical_info_hash = ?",
         )
diff --git a/src/lib.rs b/src/lib.rs
@@ -202,7 +202,7 @@
 //! torrent_info_update_interval = 3600
 //! ```
 //!
-//! For more information about configuration you can visit the documentation for the [`config`](crate::config) module.
+//! For more information about configuration you can visit the documentation for the [`config`]) module.
 //!
 //! Alternatively to the `config.toml` file you can use one environment variable `TORRUST_IDX_BACK_CONFIG` to pass the configuration to the tracker:
 //!
diff --git a/src/services/torrent.rs b/src/services/torrent.rs
@@ -170,7 +170,7 @@ impl Index {
 
         let original_info_hashes = self
             .torrent_info_hash_repository
-            .get_torrent_original_info_hashes(&canonical_info_hash)
+            .get_canonical_info_hash_group(&canonical_info_hash)
             .await?;
 
         if !original_info_hashes.is_empty() {
@@ -582,13 +582,13 @@ impl DbTorrentInfoHashRepository {
         Self { database }
     }
 
-    /// It returns all the original infohashes associated to the canonical one.
+    /// It returns all the infohashes associated to the canonical one.
     ///
     /// # Errors
     ///
     /// This function will return an error there is a database error.
-    pub async fn get_torrent_original_info_hashes(&self, info_hash: &InfoHash) -> Result<OriginalInfoHashes, Error> {
-        self.database.get_torrent_original_info_hashes(info_hash).await
+    pub async fn get_canonical_info_hash_group(&self, info_hash: &InfoHash) -> Result<OriginalInfoHashes, Error> {
+        self.database.get_torrent_canonical_info_hash_group(info_hash).await
     }
 
     /// Inserts a new infohash for the torrent. Torrents can be associated to
diff --git a/src/web/api/mod.rs b/src/web/api/mod.rs
@@ -2,7 +2,7 @@
 //!
 //! Currently, the API has only one version: `v1`.
 //!
-//! Refer to the [`v1`](crate::web::api::v1) module for more information.
+//! Refer to the [`v1`]) module for more information.
 pub mod server;
 pub mod v1;
 
diff --git a/src/web/api/v1/contexts/torrent/mod.rs b/src/web/api/v1/contexts/torrent/mod.rs
@@ -2,6 +2,32 @@
 //!
 //! This API context is responsible for handling all torrent related requests.
 //!
+//! # Original and canonical infohashes
+//!
+//! Uploaded torrents can contain non-standard fields in the `info` dictionary.
+//!
+//! For example, this is a torrent file in JSON format with a "custom" field.
+//!
+//! ```json
+//! {
+//!     "info": {
+//!        "length": 602515,
+//!        "name": "mandelbrot_set_01",
+//!        "piece length": 32768,
+//!        "pieces": "<hex>8A 88 32 BE ED 05 5F AA C4 AF 4A 90 4B 9A BF 0D EC 83 42 1C 73 39 05 B8 D6 20 2C 1B D1 8A 53 28 1F B5 D4 23 0A 23 C8 DB AC C4 E6 6B 16 12 08 C7 A4 AD 64 45 70 ED 91 0D F1 38 E7 DF 0C 1A D0 C9 23 27 7C D1 F9 D4 E5 A1 5F F5 E5 A0 E4 9E FB B1 43 F5 4B AD 0E D4 9D CB 49 F7 E6 7B BA 30 5F AF F9 88 56 FB 45 9A B4 95 92 3E 2C 7F DA A6 D3 82 E7 63 A3 BB 4B 28 F3 57 C7 CB 7D 8C 06 E3 46 AB D7 E8 8E 8A 8C 9F C7 E6 C5 C5 64 82 ED 47 BB 2A F1 B7 3F A5 3C 5B 9C AF 43 EC 2A E1 08 68 9A 49 C8 BF 1B 07 AD BE E9 2D 7E BE 9C 18 7F 4C A1 97 0E 54 3A 18 94 0E 60 8D 5C 69 0E 41 46 0D 3C 9A 37 F6 81 62 4F 95 C0 73 92 CA 9A D5 A9 89 AC 8B 85 12 53 0B FB E2 96 26 3E 26 A6 5B 70 53 48 65 F3 6C 27 0F 6B BD 1C EE EB 1A 9D 5F 77 A8 D8 AF D8 14 82 4A E0 B4 62 BC F1 A5 F5 F2 C7 60 F8 38 C8 5B 0B A9 07 DD 86 FA C0 7B F0 26 D7 D1 9A 42 C3 1F 9F B9 59 83 10 62 41 E9 06 3C 6D A1 19 75 01 57 25 9E B7 FE DF 91 04 D4 51 4B 6D 44 02 8D 31 8E 84 26 95 0F 30 31 F0 2C 16 39 BD 53 1D CF D3 5E 3E 41 A9 1E 14 3F 73 24 AC 5E 9E FC 4D C5 70 45 0F 45 8B 9B 52 E6 D0 26 47 8F 43 08 9E 2A 7C C5 92 D5 86 36 FE 48 E9 B8 86 84 92 23 49 5B EE C4 31 B2 1D 10 75 8E 4C 07 84 8F</hex>",
+//!        "custom": "custom03"
+//!     }
+//! }
+//! ```
+//!
+//! When you upload a torrent file with non-standards fields in the `info`
+//! dictionary, the Index removes those non-standard fields. That generates a
+//! new info-hash because all fields in the `info` key are used to calculate it.
+//!
+//! The Index stores the original info-hash. The resulting info-hash after
+//! removing the non-standard fields is called "canonical" infohash. The Index
+//! stores the relationship between the original info-hash and the canonical one.
+//!
 //! # Endpoints
 //!
 //! - [Upload new torrent](#upload-new-torrent)
diff --git a/src/web/api/v1/mod.rs b/src/web/api/v1/mod.rs
@@ -2,7 +2,7 @@
 //!
 //! The API is organized in contexts.
 //!
-//! Refer to the [`contexts`](crate::web::api::v1::contexts) module for more
+//! Refer to the [`contexts`] module for more
 //! information.
 pub mod auth;
 pub mod contexts;
diff --git a/tests/common/contexts/torrent/file.rs b/tests/common/contexts/torrent/file.rs
@@ -8,21 +8,21 @@ use serde::Deserialize;
 use which::which;
 
 /// Attributes parsed from a torrent file.
-#[derive(Deserialize, Clone)]
+#[derive(Deserialize, Clone, Debug)]
 pub struct TorrentFileInfo {
     pub name: String,
     pub comment: Option<String>,
-    pub creation_date: u64,
-    pub created_by: String,
+    pub creation_date: Option<u64>,
+    pub created_by: Option<String>,
     pub source: Option<String>,
     pub info_hash: String,
     pub torrent_size: u64,
     pub content_size: u64,
     pub private: bool,
     pub tracker: Option<String>,
-    pub announce_list: Vec<Vec<String>>,
+    pub announce_list: Option<Vec<Vec<String>>>,
     pub update_url: Option<String>,
-    pub dht_nodes: Vec<String>,
+    pub dht_nodes: Option<Vec<String>>,
     pub piece_size: u64,
     pub piece_count: u64,
     pub file_count: u64,
diff --git a/tests/common/contexts/torrent/fixtures.rs b/tests/common/contexts/torrent/fixtures.rs
@@ -2,7 +2,11 @@ use std::fs::File;
 use std::io::Write;
 use std::path::{Path, PathBuf};
 
+use serde::{Deserialize, Serialize};
+use serde_bytes::ByteBuf;
 use tempfile::{tempdir, TempDir};
+use torrust_index_backend::services::hasher::sha1;
+use torrust_index_backend::utils::hex::into_bytes;
 use uuid::Uuid;
 
 use super::file::{create_torrent, parse_torrent, TorrentFileInfo};
@@ -94,6 +98,45 @@ impl TestTorrent {
         }
     }
 
+    pub fn with_custom_info_dict_field(id: Uuid, file_contents: &str, custom: &str) -> Self {
+        let temp_dir = temp_dir();
+
+        let torrents_dir_path = temp_dir.path().to_owned();
+
+        // Create the torrent in memory
+        let torrent = TestTorrentWithCustomInfoField::with_contents(id, file_contents, custom);
+
+        // Bencode the torrent
+        let torrent_data = TestTorrentWithCustomInfoField::encode(&torrent).unwrap();
+
+        // Torrent temporary file path
+        let filename = format!("file-{id}.txt.torrent");
+        let torrent_path = torrents_dir_path.join(filename.clone());
+
+        // Write the torrent file to the temporary file
+        let mut file = File::create(torrent_path.clone()).unwrap();
+        file.write_all(&torrent_data).unwrap();
+
+        // Load torrent binary file
+        let torrent_file = BinaryFile::from_file_at_path(&torrent_path);
+
+        // Load torrent file metadata
+        let torrent_info = parse_torrent(&torrent_path);
+
+        let torrent_to_index = TorrentIndexInfo {
+            title: format!("title-{id}"),
+            description: format!("description-{id}"),
+            category: software_predefined_category_name(),
+            torrent_file,
+            name: filename,
+        };
+
+        TestTorrent {
+            file_info: torrent_info,
+            index_info: torrent_to_index,
+        }
+    }
+
     pub fn info_hash(&self) -> InfoHash {
         self.file_info.info_hash.clone()
     }
@@ -128,3 +171,65 @@ pub fn random_txt_file(dir: &Path, id: &Uuid) -> String {
 pub fn temp_dir() -> TempDir {
     tempdir().unwrap()
 }
+
+/// A minimal torrent file with a custom field in the info dict.
+///
+/// ```json
+/// {
+///     "info": {
+///        "length": 602515,
+///        "name": "mandelbrot_set_01",
+///        "piece length": 32768,
+///        "pieces": "<hex>8A 88 32 BE ED 05 5F AA C4 AF 4A 90 4B 9A BF 0D EC 83 42 1C 73 39 05 B8 D6 20 2C 1B D1 8A 53 28 1F B5 D4 23 0A 23 C8 DB AC C4 E6 6B 16 12 08 C7 A4 AD 64 45 70 ED 91 0D F1 38 E7 DF 0C 1A D0 C9 23 27 7C D1 F9 D4 E5 A1 5F F5 E5 A0 E4 9E FB B1 43 F5 4B AD 0E D4 9D CB 49 F7 E6 7B BA 30 5F AF F9 88 56 FB 45 9A B4 95 92 3E 2C 7F DA A6 D3 82 E7 63 A3 BB 4B 28 F3 57 C7 CB 7D 8C 06 E3 46 AB D7 E8 8E 8A 8C 9F C7 E6 C5 C5 64 82 ED 47 BB 2A F1 B7 3F A5 3C 5B 9C AF 43 EC 2A E1 08 68 9A 49 C8 BF 1B 07 AD BE E9 2D 7E BE 9C 18 7F 4C A1 97 0E 54 3A 18 94 0E 60 8D 5C 69 0E 41 46 0D 3C 9A 37 F6 81 62 4F 95 C0 73 92 CA 9A D5 A9 89 AC 8B 85 12 53 0B FB E2 96 26 3E 26 A6 5B 70 53 48 65 F3 6C 27 0F 6B BD 1C EE EB 1A 9D 5F 77 A8 D8 AF D8 14 82 4A E0 B4 62 BC F1 A5 F5 F2 C7 60 F8 38 C8 5B 0B A9 07 DD 86 FA C0 7B F0 26 D7 D1 9A 42 C3 1F 9F B9 59 83 10 62 41 E9 06 3C 6D A1 19 75 01 57 25 9E B7 FE DF 91 04 D4 51 4B 6D 44 02 8D 31 8E 84 26 95 0F 30 31 F0 2C 16 39 BD 53 1D CF D3 5E 3E 41 A9 1E 14 3F 73 24 AC 5E 9E FC 4D C5 70 45 0F 45 8B 9B 52 E6 D0 26 47 8F 43 08 9E 2A 7C C5 92 D5 86 36 FE 48 E9 B8 86 84 92 23 49 5B EE C4 31 B2 1D 10 75 8E 4C 07 84 8F</hex>",
+///        "custom": "custom03"
+///     }
+/// }
+/// ```
+///
+/// Changing the value of the `custom` field will change the info-hash of the torrent.
+#[derive(PartialEq, Debug, Clone, Serialize, Deserialize)]
+pub struct TestTorrentWithCustomInfoField {
+    pub info: InfoDictWithCustomField,
+}
+
+/// A minimal torrent info dict with a custom field.
+#[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)]
+pub struct InfoDictWithCustomField {
+    #[serde(default)]
+    pub length: i64,
+    #[serde(default)]
+    pub name: String,
+    #[serde(rename = "piece length")]
+    pub piece_length: i64,
+    #[serde(default)]
+    pub pieces: ByteBuf,
+    #[serde(default)]
+    pub custom: String,
+}
+
+impl TestTorrentWithCustomInfoField {
+    pub fn with_contents(id: Uuid, file_contents: &str, custom: &str) -> Self {
+        let sha1_of_file_contents = sha1(file_contents);
+        let pieces = into_bytes(&sha1_of_file_contents).expect("sha1 of test torrent contents cannot be converted to bytes");
+
+        Self {
+            info: InfoDictWithCustomField {
+                length: i64::try_from(file_contents.len()).expect("file contents size in bytes cannot exceed i64::MAX"),
+                name: format!("file-{id}.txt"),
+                piece_length: 16384,
+                pieces: ByteBuf::from(pieces),
+                custom: custom.to_owned(),
+            },
+        }
+    }
+
+    pub fn encode(torrent: &Self) -> Result<Vec<u8>, serde_bencode::Error> {
+        match serde_bencode::to_bytes(torrent) {
+            Ok(bencode_bytes) => Ok(bencode_bytes),
+            Err(e) => {
+                eprintln!("{e:?}");
+                Err(e)
+            }
+        }
+    }
+}
diff --git a/tests/e2e/web/api/v1/contexts/torrent/contract.rs b/tests/e2e/web/api/v1/contexts/torrent/contract.rs

Original file line number	Diff line number	Diff line change
`@@ -592,7 +592,7 @@ impl Database for Mysql {`
`592`	`592`	`}`
`593`	`593`	`}`
`594`	`594`
`595`		`- async fn get_torrent_original_info_hashes(&self, canonical: &InfoHash) -> Result<OriginalInfoHashes, database::Error> {`
	`595`	`+ async fn get_torrent_canonical_info_hash_group(&self, canonical: &InfoHash) -> Result<OriginalInfoHashes, database::Error> {`
`596`	`596`	`let db_info_hashes = query_as::<_, DbTorrentInfoHash>(`
`597`	`597`	`"SELECT info_hash, canonical_info_hash, original_is_known FROM torrust_torrent_info_hashes WHERE canonical_info_hash = ?",`
`598`	`598`	`)`
Original file line number	Diff line number	Diff line change
`@@ -582,7 +582,7 @@ impl Database for Sqlite {`
`582`	`582`	`}`
`583`	`583`	`}`
`584`	`584`
`585`		`- async fn get_torrent_original_info_hashes(&self, canonical: &InfoHash) -> Result<OriginalInfoHashes, database::Error> {`
	`585`	`+ async fn get_torrent_canonical_info_hash_group(&self, canonical: &InfoHash) -> Result<OriginalInfoHashes, database::Error> {`
`586`	`586`	`let db_info_hashes = query_as::<_, DbTorrentInfoHash>(`
`587`	`587`	`"SELECT info_hash, canonical_info_hash, original_is_known FROM torrust_torrent_info_hashes WHERE canonical_info_hash = ?",`
`588`	`588`	`)`