Fix various issues (#524)

* Fix various issues

* Fix multipart body hang

* drop req if error

* Make multipart errors more helpful
This commit is contained in:
Geometrically
2023-01-16 16:45:19 -07:00
committed by GitHub
parent 1679a3f844
commit 867ba7b68f
7 changed files with 427 additions and 357 deletions

View File

@@ -320,7 +320,7 @@ impl User {
id as UserId, id as UserId,
) )
.fetch_many(&mut *transaction) .fetch_many(&mut *transaction)
.try_filter_map(|e| async { Ok(e.right().map(|m| m.id as i64)) }) .try_filter_map(|e| async { Ok(e.right().map(|m| m.id)) })
.try_collect::<Vec<i64>>() .try_collect::<Vec<i64>>()
.await?; .await?;
@@ -442,7 +442,7 @@ impl User {
id as UserId, id as UserId,
) )
.fetch_many(&mut *transaction) .fetch_many(&mut *transaction)
.try_filter_map(|e| async { Ok(e.right().map(|m| m.id as i64)) }) .try_filter_map(|e| async { Ok(e.right().map(|m| m.id)) })
.try_collect::<Vec<i64>>() .try_collect::<Vec<i64>>()
.await?; .await?;

View File

@@ -21,7 +21,7 @@ pub async fn connect() -> Result<PgPool, sqlx::Error> {
.and_then(|x| x.parse().ok()) .and_then(|x| x.parse().ok())
.unwrap_or(16), .unwrap_or(16),
) )
.max_lifetime(Some(Duration::from_secs(60 * 60))) .max_lifetime(Some(Duration::from_secs(60 * 60 * 6)))
.connect(&database_url) .connect(&database_url)
.await?; .await?;

View File

@@ -97,7 +97,9 @@ impl PayoutsQueue {
})?; })?;
} }
let fee = if payout.recipient_wallet == *"Venmo" { let wallet = payout.recipient_wallet.clone();
let fee = if wallet == *"Venmo" {
Decimal::ONE / Decimal::from(4) Decimal::ONE / Decimal::from(4)
} else { } else {
std::cmp::min( std::cmp::min(
@@ -111,6 +113,7 @@ impl PayoutsQueue {
}; };
payout.amount.value -= fee; payout.amount.value -= fee;
payout.amount.value = payout.amount.value.round_dp(2);
if payout.amount.value <= Decimal::ZERO { if payout.amount.value <= Decimal::ZERO {
return Err(ApiError::InvalidInput( return Err(ApiError::InvalidInput(
@@ -153,7 +156,7 @@ impl PayoutsQueue {
"Error while registering payment in PayPal: {}", "Error while registering payment in PayPal: {}",
body.body.message body.body.message
))); )));
} else { } else if wallet != *"Venmo" {
#[derive(Deserialize)] #[derive(Deserialize)]
struct PayPalLink { struct PayPalLink {
href: String, href: String,

View File

@@ -11,14 +11,14 @@ use thiserror::Error;
#[derive(Debug, Error)] #[derive(Debug, Error)]
pub enum ARError { pub enum ARError {
/// Read/Write error on store /// Read/Write error on store
#[error("read/write operatiion failed: {0}")] #[error("read/write operation failed: {0}")]
ReadWrite(String), ReadWrite(String),
/// Identifier error /// Identifier error
#[error("client identification failed")] #[error("client identification failed")]
Identification, Identification,
/// Limited Error /// Limited Error
#[error("You are being ratelimited. Please wait {reset} seconds. {remaining}/{max_requests} remaining.")] #[error("You are being rate-limited. Please wait {reset} seconds. {remaining}/{max_requests} remaining.")]
Limited { Limited {
max_requests: usize, max_requests: usize,
remaining: usize, remaining: usize,

View File

@@ -11,7 +11,6 @@ use crate::search::indexing::IndexingError;
use crate::util::auth::{get_user_from_headers, AuthenticationError}; use crate::util::auth::{get_user_from_headers, AuthenticationError};
use crate::util::routes::read_from_field; use crate::util::routes::read_from_field;
use crate::util::validate::validation_errors_to_string; use crate::util::validate::validation_errors_to_string;
use actix::fut::ready;
use actix_multipart::{Field, Multipart}; use actix_multipart::{Field, Multipart};
use actix_web::http::StatusCode; use actix_web::http::StatusCode;
use actix_web::web::Data; use actix_web::web::Data;
@@ -36,8 +35,8 @@ pub enum CreateError {
DatabaseError(#[from] models::DatabaseError), DatabaseError(#[from] models::DatabaseError),
#[error("Indexing Error: {0}")] #[error("Indexing Error: {0}")]
IndexingError(#[from] IndexingError), IndexingError(#[from] IndexingError),
#[error("Error while parsing multipart payload")] #[error("Error while parsing multipart payload: {0}")]
MultipartError(actix_multipart::MultipartError), MultipartError(#[from] actix_multipart::MultipartError),
#[error("Error while parsing JSON: {0}")] #[error("Error while parsing JSON: {0}")]
SerDeError(#[from] serde_json::Error), SerDeError(#[from] serde_json::Error),
#[error("Error while validating input: {0}")] #[error("Error while validating input: {0}")]
@@ -287,9 +286,6 @@ pub async fn project_create(
let undo_result = undo_uploads(&***file_host, &uploaded_files).await; let undo_result = undo_uploads(&***file_host, &uploaded_files).await;
let rollback_result = transaction.rollback().await; let rollback_result = transaction.rollback().await;
// fix multipart error bug:
payload.for_each(|_| ready(())).await;
undo_result?; undo_result?;
if let Err(e) = rollback_result { if let Err(e) = rollback_result {
return Err(e.into()); return Err(e.into());
@@ -475,12 +471,21 @@ pub async fn project_create_inner(
let mut icon_data = None; let mut icon_data = None;
let mut error = None;
while let Some(item) = payload.next().await { while let Some(item) = payload.next().await {
let mut field: Field = item.map_err(CreateError::MultipartError)?; let mut field: Field = item?;
if error.is_some() {
continue;
}
let result = async {
let content_disposition = field.content_disposition().clone(); let content_disposition = field.content_disposition().clone();
let name = content_disposition.get_name().ok_or_else(|| { let name = content_disposition.get_name().ok_or_else(|| {
CreateError::MissingValueError("Missing content name".to_string()) CreateError::MissingValueError(
"Missing content name".to_string(),
)
})?; })?;
let (file_name, file_extension) = let (file_name, file_extension) =
@@ -504,7 +509,7 @@ pub async fn project_create_inner(
) )
.await?, .await?,
); );
continue; return Ok(());
} }
if let Some(gallery_items) = &project_create_data.gallery_items { if let Some(gallery_items) = &project_create_data.gallery_items {
@@ -514,7 +519,9 @@ pub async fn project_create_inner(
))); )));
} }
if let Some(item) = gallery_items.iter().find(|x| x.item == name) { if let Some(item) =
gallery_items.iter().find(|x| x.item == name)
{
let data = read_from_field( let data = read_from_field(
&mut field, &mut field,
5 * (1 << 20), 5 * (1 << 20),
@@ -528,7 +535,9 @@ pub async fn project_create_inner(
&content_disposition, &content_disposition,
)?; )?;
let content_type = let content_type =
crate::util::ext::get_image_content_type(file_extension) crate::util::ext::get_image_content_type(
file_extension,
)
.ok_or_else(|| { .ok_or_else(|| {
CreateError::InvalidIconFormat( CreateError::InvalidIconFormat(
file_extension.to_string(), file_extension.to_string(),
@@ -545,7 +554,7 @@ pub async fn project_create_inner(
uploaded_files.push(UploadedFile { uploaded_files.push(UploadedFile {
file_id: upload_data.file_id, file_id: upload_data.file_id,
file_name: upload_data.file_name.clone(), file_name: upload_data.file_name,
}); });
gallery_urls.push(crate::models::projects::GalleryItem { gallery_urls.push(crate::models::projects::GalleryItem {
@@ -557,7 +566,7 @@ pub async fn project_create_inner(
ordering: item.ordering, ordering: item.ordering,
}); });
continue; return Ok(());
} }
} }
@@ -597,6 +606,18 @@ pub async fn project_create_inner(
transaction, transaction,
) )
.await?; .await?;
Ok(())
}
.await;
if result.is_err() {
error = result.err();
}
}
if let Some(error) = error {
return Err(error);
} }
{ {

View File

@@ -15,7 +15,6 @@ use crate::util::auth::get_user_from_headers;
use crate::util::routes::read_from_field; use crate::util::routes::read_from_field;
use crate::util::validate::validation_errors_to_string; use crate::util::validate::validation_errors_to_string;
use crate::validate::{validate_file, ValidationResult}; use crate::validate::{validate_file, ValidationResult};
use actix::fut::ready;
use actix_multipart::{Field, Multipart}; use actix_multipart::{Field, Multipart};
use actix_web::web::Data; use actix_web::web::Data;
use actix_web::{post, web, HttpRequest, HttpResponse}; use actix_web::{post, web, HttpRequest, HttpResponse};
@@ -101,8 +100,6 @@ pub async fn version_create(
.await; .await;
let rollback_result = transaction.rollback().await; let rollback_result = transaction.rollback().await;
payload.for_each(|_| ready(())).await;
undo_result?; undo_result?;
if let Err(e) = rollback_result { if let Err(e) = rollback_result {
return Err(e.into()); return Err(e.into());
@@ -133,25 +130,33 @@ async fn version_create_inner(
let user = get_user_from_headers(req.headers(), &mut *transaction).await?; let user = get_user_from_headers(req.headers(), &mut *transaction).await?;
let mut error = None;
while let Some(item) = payload.next().await { while let Some(item) = payload.next().await {
let mut field: Field = item.map_err(CreateError::MultipartError)?; let mut field: Field = item?;
if error.is_some() {
continue;
}
let result = async {
let content_disposition = field.content_disposition().clone(); let content_disposition = field.content_disposition().clone();
let name = content_disposition.get_name().ok_or_else(|| { let name = content_disposition.get_name().ok_or_else(|| {
CreateError::MissingValueError("Missing content name".to_string()) CreateError::MissingValueError(
"Missing content name".to_string(),
)
})?; })?;
if name == "data" { if name == "data" {
let mut data = Vec::new(); let mut data = Vec::new();
while let Some(chunk) = field.next().await { while let Some(chunk) = field.next().await {
data.extend_from_slice( data.extend_from_slice(&chunk?);
&chunk.map_err(CreateError::MultipartError)?,
);
} }
let version_create_data: InitialVersionData = let version_create_data: InitialVersionData =
serde_json::from_slice(&data)?; serde_json::from_slice(&data)?;
initial_version_data = Some(version_create_data); initial_version_data = Some(version_create_data);
let version_create_data = initial_version_data.as_ref().unwrap(); let version_create_data =
initial_version_data.as_ref().unwrap();
if version_create_data.project_id.is_none() { if version_create_data.project_id.is_none() {
return Err(CreateError::MissingValueError( return Err(CreateError::MissingValueError(
"Missing project id".to_string(), "Missing project id".to_string(),
@@ -239,7 +244,8 @@ async fn version_create_inner(
}) })
.map(|y| y.id) .map(|y| y.id)
}) })
.collect::<Result<Vec<models::GameVersionId>, CreateError>>()?; .collect::<Result<Vec<models::GameVersionId>, CreateError>>(
)?;
let loaders = version_create_data let loaders = version_create_data
.loaders .loaders
@@ -252,7 +258,9 @@ async fn version_create_inner(
&& y.supported_project_types && y.supported_project_types
.contains(&project_type) .contains(&project_type)
}) })
.ok_or_else(|| CreateError::InvalidLoader(x.0.clone())) .ok_or_else(|| {
CreateError::InvalidLoader(x.0.clone())
})
.map(|y| y.id) .map(|y| y.id)
}) })
.collect::<Result<Vec<models::LoaderId>, CreateError>>()?; .collect::<Result<Vec<models::LoaderId>, CreateError>>()?;
@@ -282,13 +290,15 @@ async fn version_create_inner(
dependencies, dependencies,
game_versions, game_versions,
loaders, loaders,
version_type: version_create_data.release_channel.to_string(), version_type: version_create_data
.release_channel
.to_string(),
featured: version_create_data.featured, featured: version_create_data.featured,
status: version_create_data.status, status: version_create_data.status,
requested_status: None, requested_status: None,
}); });
continue; return Ok(());
} }
let version = version_builder.as_mut().ok_or_else(|| { let version = version_builder.as_mut().ok_or_else(|| {
@@ -309,8 +319,11 @@ async fn version_create_inner(
.await? .await?
.name; .name;
let version_data = initial_version_data.clone().ok_or_else(|| { let version_data =
CreateError::InvalidInput("`data` field is required".to_string()) initial_version_data.clone().ok_or_else(|| {
CreateError::InvalidInput(
"`data` field is required".to_string(),
)
})?; })?;
upload_file( upload_file(
@@ -334,6 +347,18 @@ async fn version_create_inner(
transaction, transaction,
) )
.await?; .await?;
Ok(())
}
.await;
if result.is_err() {
error = result.err();
}
}
if let Some(error) = error {
return Err(error);
} }
let version_data = initial_version_data.ok_or_else(|| { let version_data = initial_version_data.ok_or_else(|| {
@@ -479,8 +504,6 @@ pub async fn upload_file_to_version(
.await; .await;
let rollback_result = transaction.rollback().await; let rollback_result = transaction.rollback().await;
payload.for_each(|_| ready(())).await;
undo_result?; undo_result?;
if let Err(e) = rollback_result { if let Err(e) = rollback_result {
return Err(e.into()); return Err(e.into());
@@ -562,24 +585,31 @@ async fn upload_file_to_version_inner(
let all_game_versions = let all_game_versions =
models::categories::GameVersion::list(&mut *transaction).await?; models::categories::GameVersion::list(&mut *transaction).await?;
let mut error = None;
while let Some(item) = payload.next().await { while let Some(item) = payload.next().await {
let mut field: Field = item.map_err(CreateError::MultipartError)?; let mut field: Field = item?;
if error.is_some() {
continue;
}
let result = async {
let content_disposition = field.content_disposition().clone(); let content_disposition = field.content_disposition().clone();
let name = content_disposition.get_name().ok_or_else(|| { let name = content_disposition.get_name().ok_or_else(|| {
CreateError::MissingValueError("Missing content name".to_string()) CreateError::MissingValueError(
"Missing content name".to_string(),
)
})?; })?;
if name == "data" { if name == "data" {
let mut data = Vec::new(); let mut data = Vec::new();
while let Some(chunk) = field.next().await { while let Some(chunk) = field.next().await {
data.extend_from_slice( data.extend_from_slice(&chunk?);
&chunk.map_err(CreateError::MultipartError)?,
);
} }
let file_data: InitialFileData = serde_json::from_slice(&data)?; let file_data: InitialFileData = serde_json::from_slice(&data)?;
initial_file_data = Some(file_data); initial_file_data = Some(file_data);
continue; return Ok(());
} }
let file_data = initial_file_data.as_ref().ok_or_else(|| { let file_data = initial_file_data.as_ref().ok_or_else(|| {
@@ -625,6 +655,18 @@ async fn upload_file_to_version_inner(
transaction, transaction,
) )
.await?; .await?;
Ok(())
}
.await;
if result.is_err() {
error = result.err();
}
}
if let Some(error) = error {
return Err(error);
} }
if file_builders.is_empty() { if file_builders.is_empty() {
@@ -665,6 +707,12 @@ pub async fn upload_file(
) -> Result<(), CreateError> { ) -> Result<(), CreateError> {
let (file_name, file_extension) = get_name_ext(content_disposition)?; let (file_name, file_extension) = get_name_ext(content_disposition)?;
if file_name.contains('/') {
return Err(CreateError::InvalidInput(
"File names must not contain slashes!".to_string(),
));
}
let content_type = crate::util::ext::project_file_type(file_extension) let content_type = crate::util::ext::project_file_type(file_extension)
.ok_or_else(|| { .ok_or_else(|| {
CreateError::InvalidFileType(file_extension.to_string()) CreateError::InvalidFileType(file_extension.to_string())

View File

@@ -37,9 +37,7 @@ pub async fn read_from_field(
if bytes.len() >= cap { if bytes.len() >= cap {
return Err(CreateError::InvalidInput(String::from(err_msg))); return Err(CreateError::InvalidInput(String::from(err_msg)));
} else { } else {
bytes.extend_from_slice( bytes.extend_from_slice(&chunk?);
&chunk.map_err(CreateError::MultipartError)?,
);
} }
} }
Ok(bytes) Ok(bytes)