Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Bad Request error message for uploading #259

Merged
merged 1 commit into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 2 additions & 10 deletions src/models/torrent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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(())
}
Expand Down
1 change: 1 addition & 0 deletions src/models/torrent_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
77 changes: 55 additions & 22 deletions src/services/torrent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}

Expand Down
61 changes: 61 additions & 0 deletions src/web/api/v1/contexts/torrent/errors.rs
Original file line number Diff line number Diff line change
@@ -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,
}
}
Loading