Compare commits

..

5 Commits

Author SHA1 Message Date
Dessalines
0eb51bbbfb
Merge 846a6c56cd into 777992e829 2024-09-30 22:11:19 -04:00
renovate[bot]
777992e829
Update Rust crate reqwest to v0.12.8 (#5068)
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
2024-09-30 22:04:24 -04:00
renovate[bot]
0fcbd25ad5
Update Rust crate clap to v4.5.18 (#5066)
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
2024-09-30 21:42:11 -04:00
Nutomic
44dda08b13
Avoid stack overflow when fetching nested comments, reduce max comment depth to 50 (#5009)
* Avoid stack overflow when fetching deeply nested comments

* add test case

* reduce comment depth, add docs

* decrease

* reduce max comment depth to 50

* fmt

* clippy

* cleanup
2024-09-30 20:27:14 -04:00
Nutomic
5115ed4c09
Handle partial settings backup (fixes #4307) (#5063)
* Handle partial settings backup (fixes #4307)

* clippy
2024-09-30 20:21:06 -04:00
8 changed files with 101 additions and 73 deletions

40
Cargo.lock generated
View File

@ -42,7 +42,7 @@ dependencies = [
"pin-project-lite",
"rand",
"regex",
"reqwest 0.12.7",
"reqwest 0.12.8",
"reqwest-middleware",
"rsa",
"serde",
@ -839,9 +839,9 @@ dependencies = [
[[package]]
name = "clap"
version = "4.5.17"
version = "4.5.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3e5a21b8495e732f1b3c364c9949b201ca7bae518c502c80256c96ad79eaf6ac"
checksum = "b0956a43b323ac1afaffc053ed5c4b7c1f1800bacd1683c353aabbb752515dd3"
dependencies = [
"clap_builder",
"clap_derive",
@ -849,9 +849,9 @@ dependencies = [
[[package]]
name = "clap_builder"
version = "4.5.17"
version = "4.5.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8cf2dd12af7a047ad9d6da2b6b249759a22a7abc0f474c1dae1777afa4b21a73"
checksum = "4d72166dd41634086d5803a47eb71ae740e61d84709c36f3c34110173db3961b"
dependencies = [
"anstream",
"anstyle",
@ -861,9 +861,9 @@ dependencies = [
[[package]]
name = "clap_derive"
version = "4.5.13"
version = "4.5.18"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "501d359d5f3dcaf6ecdeee48833ae73ec6e42723a1e52419c79abf9507eec0a0"
checksum = "4ac6a0c7b1a9e9a5186361f67dfa1b88213572f427fb9ab038efb2bd8c582dab"
dependencies = [
"heck 0.5.0",
"proc-macro2",
@ -1989,7 +1989,7 @@ dependencies = [
"base64 0.22.1",
"http-signature-normalization",
"httpdate",
"reqwest 0.12.7",
"reqwest 0.12.8",
"reqwest-middleware",
"sha2",
"thiserror",
@ -2514,7 +2514,7 @@ dependencies = [
"moka",
"pretty_assertions",
"regex",
"reqwest 0.12.7",
"reqwest 0.12.8",
"reqwest-middleware",
"rosetta-i18n",
"serde",
@ -2579,7 +2579,7 @@ dependencies = [
"lemmy_utils",
"moka",
"pretty_assertions",
"reqwest 0.12.7",
"reqwest 0.12.8",
"serde",
"serde_json",
"serde_with",
@ -2718,7 +2718,7 @@ dependencies = [
"lemmy_utils",
"mockall",
"moka",
"reqwest 0.12.7",
"reqwest 0.12.8",
"serde_json",
"serial_test",
"test-context",
@ -2745,7 +2745,7 @@ dependencies = [
"lemmy_db_views",
"lemmy_db_views_actor",
"lemmy_utils",
"reqwest 0.12.7",
"reqwest 0.12.8",
"reqwest-middleware",
"rss",
"serde",
@ -2778,7 +2778,7 @@ dependencies = [
"lemmy_utils",
"pretty_assertions",
"prometheus",
"reqwest 0.12.7",
"reqwest 0.12.8",
"reqwest-middleware",
"reqwest-tracing",
"rustls 0.23.13",
@ -2811,7 +2811,7 @@ dependencies = [
"markdown-it",
"pretty_assertions",
"regex",
"reqwest 0.12.7",
"reqwest 0.12.8",
"reqwest-middleware",
"rosetta-build",
"rosetta-i18n",
@ -2870,7 +2870,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4979f22fdb869068da03c9f7528f8297c6fd2606bc3a4affe42e6a823fdb8da4"
dependencies = [
"cfg-if",
"windows-targets 0.52.6",
"windows-targets 0.48.5",
]
[[package]]
@ -3966,9 +3966,9 @@ dependencies = [
[[package]]
name = "reqwest"
version = "0.12.7"
version = "0.12.8"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f8f4955649ef5c38cc7f9e8aa41761d48fb9677197daea9984dc54f56aad5e63"
checksum = "f713147fbe92361e52392c73b8c9e48c04c6625bce969ef54dc901e58e042a7b"
dependencies = [
"async-compression",
"base64 0.22.1",
@ -4019,7 +4019,7 @@ dependencies = [
"anyhow",
"async-trait",
"http 1.1.0",
"reqwest 0.12.7",
"reqwest 0.12.8",
"serde",
"thiserror",
"tower-service",
@ -4036,7 +4036,7 @@ dependencies = [
"getrandom",
"http 1.1.0",
"matchit",
"reqwest 0.12.7",
"reqwest 0.12.8",
"reqwest-middleware",
"tracing",
]
@ -5579,7 +5579,7 @@ version = "0.1.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb"
dependencies = [
"windows-sys 0.59.0",
"windows-sys 0.48.0",
]
[[package]]

View File

@ -90,7 +90,7 @@ lemmy_db_views = { version = "=0.19.6-beta.7", path = "./crates/db_views" }
lemmy_db_views_actor = { version = "=0.19.6-beta.7", path = "./crates/db_views_actor" }
lemmy_db_views_moderator = { version = "=0.19.6-beta.7", path = "./crates/db_views_moderator" }
lemmy_federate = { version = "=0.19.6-beta.7", path = "./crates/federate" }
activitypub_federation = { version = "0.6.0-alpha1", default-features = false, features = [
activitypub_federation = { version = "0.6.0-alpha2", default-features = false, features = [
"actix-web",
] }
diesel = "2.1.6"

View File

@ -858,3 +858,26 @@ test("Dont send a comment reply to a blocked community", async () => {
blockRes = await blockCommunity(beta, newCommunityId, false);
expect(blockRes.blocked).toBe(false);
});
/// Fetching a deeply nested comment can lead to stack overflow as all parent comments are also
/// fetched recursively. Ensure that it works properly.
test("Fetch a deeply nested comment", async () => {
let lastComment;
for (let i = 0; i < 50; i++) {
let commentRes = await createComment(
alpha,
postOnAlphaRes.post_view.post.id,
lastComment?.comment_view.comment.id,
);
expect(commentRes.comment_view.comment).toBeDefined();
lastComment = commentRes;
}
let betaComment = await resolveComment(
beta,
lastComment!.comment_view.comment,
);
expect(betaComment!.comment!.comment).toBeDefined();
expect(betaComment?.comment?.post).toBeDefined();
});

View File

@ -30,10 +30,9 @@ use lemmy_db_views::structs::{LocalUserView, PostView};
use lemmy_utils::{
error::{LemmyErrorExt, LemmyErrorType, LemmyResult},
utils::{mention::scrape_text_for_mentions, validation::is_valid_body_field},
MAX_COMMENT_DEPTH_LIMIT,
};
const MAX_COMMENT_DEPTH_LIMIT: usize = 100;
#[tracing::instrument(skip(context))]
pub async fn create_comment(
data: Json<CreateComment>,

View File

@ -103,13 +103,16 @@ pub async fn import_settings(
context: Data<LemmyContext>,
) -> LemmyResult<Json<SuccessResponse>> {
let person_form = PersonUpdateForm {
display_name: Some(data.display_name.clone()),
bio: Some(data.bio.clone()),
matrix_user_id: Some(data.matrix_id.clone()),
display_name: data.display_name.clone().map(Some),
bio: data.bio.clone().map(Some),
matrix_user_id: data.bio.clone().map(Some),
bot_account: data.bot_account,
..Default::default()
};
Person::update(&mut context.pool(), local_user_view.person.id, &person_form).await?;
// ignore error in case form is empty
Person::update(&mut context.pool(), local_user_view.person.id, &person_form)
.await
.ok();
let local_user_form = LocalUserUpdateForm {
show_nsfw: data.settings.as_ref().map(|s| s.show_nsfw),
@ -312,8 +315,9 @@ where
#[expect(clippy::indexing_slicing)]
mod tests {
use crate::api::user_settings_backup::{export_settings, import_settings, UserSettingsBackup};
use crate::api::user_settings_backup::{export_settings, import_settings};
use activitypub_federation::config::Data;
use actix_web::web::Json;
use lemmy_api_common::context::LemmyContext;
use lemmy_db_schema::{
source::{
@ -401,45 +405,6 @@ mod tests {
Ok(())
}
#[tokio::test]
#[serial]
async fn test_settings_partial_import() -> LemmyResult<()> {
let context = LemmyContext::init_test_context().await;
let export_user =
create_user("hanna".to_string(), Some("my bio".to_string()), &context).await?;
let community_form = CommunityInsertForm::new(
export_user.person.instance_id,
"testcom".to_string(),
"testcom".to_string(),
"pubkey".to_string(),
);
let community = Community::create(&mut context.pool(), &community_form).await?;
let follower_form = CommunityFollowerForm {
community_id: community.id,
person_id: export_user.person.id,
pending: false,
};
CommunityFollower::follow(&mut context.pool(), &follower_form).await?;
let backup = export_settings(export_user.clone(), context.reset_request_count()).await?;
let import_user = create_user("charles".to_string(), None, &context).await?;
let backup2 = UserSettingsBackup {
followed_communities: backup.followed_communities.clone(),
..Default::default()
};
import_settings(
actix_web::web::Json(backup2),
import_user.clone(),
context.reset_request_count(),
)
.await?;
Ok(())
}
#[tokio::test]
#[serial]
async fn disallow_large_backup() -> LemmyResult<()> {
@ -475,4 +440,33 @@ mod tests {
LocalUser::delete(&mut context.pool(), import_user.local_user.id).await?;
Ok(())
}
#[tokio::test]
#[serial]
async fn import_partial_backup() -> LemmyResult<()> {
let context = LemmyContext::init_test_context().await;
let import_user =
create_user("hanna".to_string(), Some("my bio".to_string()), &context).await?;
let backup =
serde_json::from_str("{\"bot_account\": true, \"settings\": {\"theme\": \"my_theme\"}}")?;
import_settings(
Json(backup),
import_user.clone(),
context.reset_request_count(),
)
.await?;
let import_user_updated =
LocalUserView::read(&mut context.pool(), import_user.local_user.id).await?;
// mark as bot account
assert!(import_user_updated.person.bot_account);
// dont remove existing bio
assert_eq!(import_user.person.bio, import_user_updated.person.bio);
// local_user can be deserialized without id/person_id fields
assert_eq!("my_theme", import_user_updated.local_user.theme);
Ok(())
}
}

View File

@ -20,10 +20,9 @@ use lemmy_db_schema::{
source::{community::Community, post::Post},
traits::Crud,
};
use lemmy_utils::error::LemmyResult;
use lemmy_utils::{error::LemmyResult, LemmyErrorType, MAX_COMMENT_DEPTH_LIMIT};
use serde::{Deserialize, Serialize};
use serde_with::skip_serializing_none;
use std::ops::Deref;
use url::Url;
#[skip_serializing_none]
@ -58,9 +57,19 @@ impl Note {
&self,
context: &Data<LemmyContext>,
) -> LemmyResult<(ApubPost, Option<ApubComment>)> {
// Fetch parent comment chain in a box, otherwise it can cause a stack overflow.
let parent = Box::pin(self.in_reply_to.dereference(context).await?);
match parent.deref() {
// We use recursion here to fetch the entire comment chain up to the top-level parent. This is
// necessary because we need to know the post and parent comment in order to insert a new
// comment. However it can also lead to stack overflow when fetching many comments recursively.
// To avoid this we check the request count against max comment depth, which based on testing
// can be handled without risking stack overflow. This is not a perfect solution, because in
// some cases we have to fetch user profiles too, and reach the limit after only 25 comments
// or so.
// A cleaner solution would be converting the recursion into a loop, but that is tricky.
if context.request_count() > MAX_COMMENT_DEPTH_LIMIT as u32 {
Err(LemmyErrorType::MaxCommentDepthReached)?;
}
let parent = self.in_reply_to.dereference(context).await?;
match parent {
PostOrComment::Post(p) => Ok((p.clone(), None)),
PostOrComment::Comment(c) => {
let post_id = c.post_id;

View File

@ -14,11 +14,12 @@ use serde_with::skip_serializing_none;
use ts_rs::TS;
#[skip_serializing_none]
#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)]
#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Default)]
#[cfg_attr(feature = "full", derive(Queryable, Selectable, Identifiable, TS))]
#[cfg_attr(feature = "full", diesel(table_name = local_user))]
#[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))]
#[cfg_attr(feature = "full", ts(export))]
#[serde(default)]
/// A local user.
pub struct LocalUser {
pub id: LocalUserId,

View File

@ -29,6 +29,8 @@ pub const CACHE_DURATION_FEDERATION: Duration = Duration::from_secs(60);
pub const CACHE_DURATION_API: Duration = Duration::from_secs(1);
pub const MAX_COMMENT_DEPTH_LIMIT: usize = 50;
#[macro_export]
macro_rules! location_info {
() => {