Merge #259: Improve Bad Request error message for uploading

ce50f26 feat: [#257] improve bad request error message for uploading (Jose Celano)

Pull request description:

  Add a custom error for each case so that the Bad Request response constains a descriptive error message.

Top commit has no ACKs.

Tree-SHA512: 70659460ad69a694da21e66416bb51d2a42b743f6ed16a0d15fb460197b98b71921de740b707a50e73ce9e0d7239f22e92cd252b97a87af0bf3975d4043dd757

Author: josecelano

src/errors.rs

@@ -97,8 +97,8 @@ pub enum ServiceError {
     #[display(fmt = "Torrent title is too short.")]
     InvalidTorrentTitleLength,
 
-    #[display(fmt = "Bad request.")]
-    BadRequest,
+    #[display(fmt = "Some mandatory metadata fields are missing.")]
+    MissingMandatoryMetadataFields,
 
     #[display(fmt = "Selected category does not exist.")]
     InvalidCategory,
@@ -223,7 +223,7 @@ pub fn http_status_code_for_service_error(error: &ServiceError) -> StatusCode {
         ServiceError::InvalidTorrentPiecesLength => StatusCode::BAD_REQUEST,
         ServiceError::InvalidFileType => StatusCode::BAD_REQUEST,
         ServiceError::InvalidTorrentTitleLength => StatusCode::BAD_REQUEST,
-        ServiceError::BadRequest => StatusCode::BAD_REQUEST,
+        ServiceError::MissingMandatoryMetadataFields => StatusCode::BAD_REQUEST,
         ServiceError::InvalidCategory => StatusCode::BAD_REQUEST,
         ServiceError::InvalidTag => StatusCode::BAD_REQUEST,
         ServiceError::Unauthorized => StatusCode::FORBIDDEN,

src/models/torrent.rs

@@ -2,7 +2,6 @@ use serde::{Deserialize, Serialize};
 
 use super::torrent_tag::TagId;
 use crate::errors::ServiceError;
-use crate::models::torrent_file::Torrent;
 
 #[allow(clippy::module_name_repetitions)]
 pub type TorrentId = i64;
@@ -23,13 +22,6 @@ pub struct TorrentListing {
     pub leechers: i64,
 }
 
-#[allow(clippy::module_name_repetitions)]
-#[derive(Debug)]
-pub struct AddTorrentRequest {
-    pub metadata: Metadata,
-    pub torrent: Torrent,
-}
-
 #[derive(Debug, Deserialize)]
 pub struct Metadata {
     pub title: String,
@@ -43,10 +35,10 @@ impl Metadata {
     ///
     /// # Errors
     ///
-    /// This function will return an `BadRequest` error if the `title` or the `category` is empty.
+    /// This function will return an error if the any of the mandatory metadata fields are missing.
     pub fn verify(&self) -> Result<(), ServiceError> {
         if self.title.is_empty() || self.category.is_empty() {
-            Err(ServiceError::BadRequest)
+            Err(ServiceError::MissingMandatoryMetadataFields)
         } else {
             Ok(())
         }

src/models/torrent_file.rs

@@ -229,6 +229,7 @@ impl Torrent {
 
     #[must_use]
     pub fn info_hash(&self) -> String {
+        // todo: return an InfoHash struct
         from_bytes(&self.calculate_info_hash_as_bytes()).to_lowercase()
     }
 

src/services/torrent.rs

@@ -11,11 +11,12 @@ use crate::errors::ServiceError;
 use crate::models::category::CategoryId;
 use crate::models::info_hash::InfoHash;
 use crate::models::response::{DeletedTorrentResponse, TorrentResponse, TorrentsResponse};
-use crate::models::torrent::{AddTorrentRequest, TorrentId, TorrentListing};
+use crate::models::torrent::{Metadata, TorrentId, TorrentListing};
 use crate::models::torrent_file::{DbTorrentInfo, Torrent, TorrentFile};
 use crate::models::torrent_tag::{TagId, TorrentTag};
 use crate::models::user::UserId;
 use crate::tracker::statistics_importer::StatisticsImporter;
+use crate::utils::parse_torrent;
 use crate::{tracker, AsCSV};
 
 const MIN_TORRENT_TITLE_LENGTH: usize = 3;
@@ -34,6 +35,14 @@ pub struct Index {
     torrent_listing_generator: Arc<DbTorrentListingGenerator>,
 }
 
+pub struct AddTorrentRequest {
+    pub title: String,
+    pub description: String,
+    pub category: String,
+    pub tags: Vec<TagId>,
+    pub torrent_buffer: Vec<u8>,
+}
+
 /// User request to generate a torrent listing.
 #[derive(Debug, Deserialize)]
 pub struct ListingRequest {
@@ -101,46 +110,75 @@ impl Index {
     /// * Unable to insert the torrent into the database.
     /// * Unable to add the torrent to the whitelist.
     /// * Torrent title is too short.
-    pub async fn add_torrent(&self, mut torrent_request: AddTorrentRequest, user_id: UserId) -> Result<TorrentId, ServiceError> {
+    ///
+    /// # Panics
+    ///
+    /// This function will panic if:
+    ///
+    /// * Unable to parse the torrent info-hash.
+    pub async fn add_torrent(
+        &self,
+        add_torrent_form: AddTorrentRequest,
+        user_id: UserId,
+    ) -> Result<(TorrentId, InfoHash), ServiceError> {
+        let metadata = Metadata {
+            title: add_torrent_form.title,
+            description: add_torrent_form.description,
+            category: add_torrent_form.category,
+            tags: add_torrent_form.tags,
+        };
+
+        metadata.verify()?;
+
+        let mut torrent =
+            parse_torrent::decode_torrent(&add_torrent_form.torrent_buffer).map_err(|_| ServiceError::InvalidTorrentFile)?;
+
+        // Make sure that the pieces key has a length that is a multiple of 20
+        if let Some(pieces) = torrent.info.pieces.as_ref() {
+            if pieces.as_ref().len() % 20 != 0 {
+                return Err(ServiceError::InvalidTorrentPiecesLength);
+            }
+        }
+
         let _user = self.user_repository.get_compact(&user_id).await?;
 
-        torrent_request.torrent.set_announce_urls(&self.configuration).await;
+        torrent.set_announce_urls(&self.configuration).await;
 
-        if torrent_request.metadata.title.len() < MIN_TORRENT_TITLE_LENGTH {
+        if metadata.title.len() < MIN_TORRENT_TITLE_LENGTH {
             return Err(ServiceError::InvalidTorrentTitleLength);
         }
 
         let category = self
             .category_repository
-            .get_by_name(&torrent_request.metadata.category)
+            .get_by_name(&metadata.category)
             .await
             .map_err(|_| ServiceError::InvalidCategory)?;
 
-        let torrent_id = self.torrent_repository.add(&torrent_request, user_id, category).await?;
+        let torrent_id = self.torrent_repository.add(&torrent, &metadata, user_id, category).await?;
+        let info_hash: InfoHash = torrent
+            .info_hash()
+            .parse()
+            .expect("the parsed torrent should have a valid info hash");
 
         drop(
             self.tracker_statistics_importer
-                .import_torrent_statistics(torrent_id, &torrent_request.torrent.info_hash())
+                .import_torrent_statistics(torrent_id, &torrent.info_hash())
                 .await,
         );
 
         // We always whitelist the torrent on the tracker because even if the tracker mode is `public`
         // it could be changed to `private` later on.
-        if let Err(e) = self
-            .tracker_service
-            .whitelist_info_hash(torrent_request.torrent.info_hash())
-            .await
-        {
+        if let Err(e) = self.tracker_service.whitelist_info_hash(torrent.info_hash()).await {
             // If the torrent can't be whitelisted somehow, remove the torrent from database
             drop(self.torrent_repository.delete(&torrent_id).await);
             return Err(e);
         }
 
         self.torrent_tag_repository
-            .link_torrent_to_tags(&torrent_id, &torrent_request.metadata.tags)
+            .link_torrent_to_tags(&torrent_id, &metadata.tags)
             .await?;
 
-        Ok(torrent_id)
+        Ok((torrent_id, info_hash))
     }
 
     /// Gets a torrent from the Index.
@@ -436,18 +474,13 @@ impl DbTorrentRepository {
     /// This function will return an error there is a database error.
     pub async fn add(
         &self,
-        torrent_request: &AddTorrentRequest,
+        torrent: &Torrent,
+        metadata: &Metadata,
         user_id: UserId,
         category: Category,
     ) -> Result<TorrentId, Error> {
         self.database
-            .insert_torrent_and_get_id(
-                &torrent_request.torrent,
-                user_id,
-                category.category_id,
-                &torrent_request.metadata.title,
-                &torrent_request.metadata.description,
-            )
+            .insert_torrent_and_get_id(torrent, user_id, category.category_id, &metadata.title, &metadata.description)
             .await
     }
 

src/web/api/v1/contexts/torrent/errors.rs

@@ -0,0 +1,61 @@
+use axum::response::{IntoResponse, Response};
+use derive_more::{Display, Error};
+use hyper::StatusCode;
+
+use crate::web::api::v1::responses::{json_error_response, ErrorResponseData};
+
+#[derive(Debug, Display, PartialEq, Eq, Error)]
+pub enum Request {
+    #[display(fmt = "torrent title bytes are nota valid UTF8 string.")]
+    TitleIsNotValidUtf8,
+
+    #[display(fmt = "torrent description bytes are nota valid UTF8 string.")]
+    DescriptionIsNotValidUtf8,
+
+    #[display(fmt = "torrent category bytes are nota valid UTF8 string.")]
+    CategoryIsNotValidUtf8,
+
+    #[display(fmt = "torrent tags arrays bytes are nota valid UTF8 string array.")]
+    TagsArrayIsNotValidUtf8,
+
+    #[display(fmt = "torrent tags string is not a valid JSON.")]
+    TagsArrayIsNotValidJson,
+
+    #[display(fmt = "upload torrent request header `content-type` should be `application/x-bittorrent`.")]
+    InvalidFileType,
+
+    #[display(fmt = "cannot write uploaded torrent bytes (binary file) into memory.")]
+    CannotWriteChunkFromUploadedBinary,
+
+    #[display(fmt = "cannot read a chunk of bytes from the uploaded torrent file. Review the request body size limit.")]
+    CannotReadChunkFromUploadedBinary,
+
+    #[display(fmt = "provided path param for Info-hash is not valid.")]
+    InvalidInfoHashParam,
+}
+
+impl IntoResponse for Request {
+    fn into_response(self) -> Response {
+        json_error_response(
+            http_status_code_for_handler_error(&self),
+            &ErrorResponseData { error: self.to_string() },
+        )
+    }
+}
+
+#[must_use]
+pub fn http_status_code_for_handler_error(error: &Request) -> StatusCode {
+    #[allow(clippy::match_same_arms)]
+    match error {
+        Request::TitleIsNotValidUtf8 => StatusCode::BAD_REQUEST,
+        Request::DescriptionIsNotValidUtf8 => StatusCode::BAD_REQUEST,
+        Request::CategoryIsNotValidUtf8 => StatusCode::BAD_REQUEST,
+        Request::TagsArrayIsNotValidUtf8 => StatusCode::BAD_REQUEST,
+        Request::TagsArrayIsNotValidJson => StatusCode::BAD_REQUEST,
+        Request::InvalidFileType => StatusCode::BAD_REQUEST,
+        Request::InvalidInfoHashParam => StatusCode::BAD_REQUEST,
+        // Internal errors processing the request
+        Request::CannotWriteChunkFromUploadedBinary => StatusCode::INTERNAL_SERVER_ERROR,
+        Request::CannotReadChunkFromUploadedBinary => StatusCode::INTERNAL_SERVER_ERROR,
+    }
+}