diff --git a/Cargo.lock b/Cargo.lock index 18b6b3082c..154c684a6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2699,7 +2699,7 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" [[package]] name = "lemmy_api" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "activitypub_federation", "actix-web", @@ -2728,7 +2728,7 @@ dependencies = [ [[package]] name = "lemmy_api_common" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "activitypub_federation", "actix-web", @@ -2766,7 +2766,7 @@ dependencies = [ [[package]] name = "lemmy_api_crud" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "accept-language", "activitypub_federation", @@ -2789,7 +2789,7 @@ dependencies = [ [[package]] name = "lemmy_apub" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "activitypub_federation", "actix-web", @@ -2827,7 +2827,7 @@ dependencies = [ [[package]] name = "lemmy_db_perf" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "anyhow", "clap", @@ -2842,7 +2842,7 @@ dependencies = [ [[package]] name = "lemmy_db_schema" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "activitypub_federation", "anyhow", @@ -2882,7 +2882,7 @@ dependencies = [ [[package]] name = "lemmy_db_views" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "actix-web", "chrono", @@ -2904,7 +2904,7 @@ dependencies = [ [[package]] name = "lemmy_db_views_actor" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "chrono", "diesel", @@ -2925,7 +2925,7 @@ dependencies = [ [[package]] name = "lemmy_db_views_moderator" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "diesel", "diesel-async", @@ -2937,7 +2937,7 @@ dependencies = [ [[package]] name = "lemmy_federate" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "activitypub_federation", "anyhow", @@ -2962,7 +2962,7 @@ dependencies = [ [[package]] name = "lemmy_routes" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "activitypub_federation", "actix-web", @@ -2987,7 +2987,7 @@ dependencies = [ [[package]] name = "lemmy_server" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "activitypub_federation", "actix-cors", @@ -3030,7 +3030,7 @@ dependencies = [ [[package]] name = "lemmy_utils" -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" dependencies = [ "actix-web", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 60babeb1d0..472b61692d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [workspace.package] -version = "0.19.4-rc.5" +version = "0.19.4-rc.6" edition = "2021" description = "A link aggregator for the fediverse" license = "AGPL-3.0" @@ -88,17 +88,17 @@ unused_self = "deny" unwrap_used = "deny" [workspace.dependencies] -lemmy_api = { version = "=0.19.4-rc.5", path = "./crates/api" } -lemmy_api_crud = { version = "=0.19.4-rc.5", path = "./crates/api_crud" } -lemmy_apub = { version = "=0.19.4-rc.5", path = "./crates/apub" } -lemmy_utils = { version = "=0.19.4-rc.5", path = "./crates/utils", default-features = false } -lemmy_db_schema = { version = "=0.19.4-rc.5", path = "./crates/db_schema" } -lemmy_api_common = { version = "=0.19.4-rc.5", path = "./crates/api_common" } -lemmy_routes = { version = "=0.19.4-rc.5", path = "./crates/routes" } -lemmy_db_views = { version = "=0.19.4-rc.5", path = "./crates/db_views" } -lemmy_db_views_actor = { version = "=0.19.4-rc.5", path = "./crates/db_views_actor" } -lemmy_db_views_moderator = { version = "=0.19.4-rc.5", path = "./crates/db_views_moderator" } -lemmy_federate = { version = "=0.19.4-rc.5", path = "./crates/federate" } +lemmy_api = { version = "=0.19.4-rc.6", path = "./crates/api" } +lemmy_api_crud = { version = "=0.19.4-rc.6", path = "./crates/api_crud" } +lemmy_apub = { version = "=0.19.4-rc.6", path = "./crates/apub" } +lemmy_utils = { version = "=0.19.4-rc.6", path = "./crates/utils", default-features = false } +lemmy_db_schema = { version = "=0.19.4-rc.6", path = "./crates/db_schema" } +lemmy_api_common = { version = "=0.19.4-rc.6", path = "./crates/api_common" } +lemmy_routes = { version = "=0.19.4-rc.6", path = "./crates/routes" } +lemmy_db_views = { version = "=0.19.4-rc.6", path = "./crates/db_views" } +lemmy_db_views_actor = { version = "=0.19.4-rc.6", path = "./crates/db_views_actor" } +lemmy_db_views_moderator = { version = "=0.19.4-rc.6", path = "./crates/db_views_moderator" } +lemmy_federate = { version = "=0.19.4-rc.6", path = "./crates/federate" } activitypub_federation = { version = "0.5.6", default-features = false, features = [ "actix-web", ] } diff --git a/api_tests/src/user.spec.ts b/api_tests/src/user.spec.ts index f44f3cc0a8..4f91cbd878 100644 --- a/api_tests/src/user.spec.ts +++ b/api_tests/src/user.spec.ts @@ -186,10 +186,16 @@ test("Set a new avatar, old avatar is deleted", async () => { expect(upload2.url).toBeDefined(); let form2 = { - avatar: upload1.url, + avatar: upload2.url, }; await saveUserSettings(alpha, form2); // make sure only the new avatar is kept const listMediaRes2 = await alphaImage.listMedia(); expect(listMediaRes2.images.length).toBe(1); + + // Upload that same form2 avatar, make sure it isn't replaced / deleted + await saveUserSettings(alpha, form2); + // make sure only the new avatar is kept + const listMediaRes3 = await alphaImage.listMedia(); + expect(listMediaRes3.images.length).toBe(1); }); diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index 96e0d7868e..62c6189164 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -345,16 +345,19 @@ async fn is_image_content_type(client: &ClientWithMiddleware, url: &Url) -> Lemm } } -/// When adding a new avatar or similar image, delete the old one. +/// When adding a new avatar, banner or similar image, delete the old one. pub async fn replace_image( new_image: &Option, old_image: &Option, context: &Data, ) -> LemmyResult<()> { - if new_image.is_some() { - // Ignore errors because image may be stored externally. - if let Some(avatar) = &old_image { - let image = LocalImage::delete_by_url(&mut context.pool(), avatar) + if let (Some(new_image), Some(old_image)) = (new_image, old_image) { + // Note: Oftentimes front ends will include the current image in the form. + // In this case, deleting `old_image` would also be deletion of `new_image`, + // so the deletion must be skipped for the image to be kept. + if new_image != old_image.as_str() { + // Ignore errors because image may be stored externally. + let image = LocalImage::delete_by_url(&mut context.pool(), old_image) .await .ok(); if let Some(image) = image { diff --git a/crates/routes/src/nodeinfo.rs b/crates/routes/src/nodeinfo.rs index 17ea209482..bcb835309d 100644 --- a/crates/routes/src/nodeinfo.rs +++ b/crates/routes/src/nodeinfo.rs @@ -12,15 +12,18 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; use url::Url; +/// A description of the nodeinfo endpoint is here: +/// https://github.com/jhass/nodeinfo/blob/main/PROTOCOL.md pub fn config(cfg: &mut web::ServiceConfig) { cfg .route( - "/nodeinfo/2.1.json", + "/nodeinfo/2.1", web::get().to(node_info).wrap(cache_1hour()), ) - .service(web::redirect("/version", "/nodeinfo/2.1.json")) + .service(web::redirect("/version", "/nodeinfo/2.1")) // For backwards compatibility, can be removed after Lemmy 0.20 - .service(web::redirect("/nodeinfo/2.0.json", "/nodeinfo/2.1.json")) + .service(web::redirect("/nodeinfo/2.0.json", "/nodeinfo/2.1")) + .service(web::redirect("/nodeinfo/2.1.json", "/nodeinfo/2.1")) .route( "/.well-known/nodeinfo", web::get().to(node_info_well_known).wrap(cache_3days()), @@ -32,7 +35,7 @@ async fn node_info_well_known(context: web::Data) -> LemmyResult) -> Result, } #[derive(Serialize, Deserialize, Debug)] -struct NodeInfoWellKnownLinks { +pub struct NodeInfoWellKnownLinks { pub rel: Url, pub href: Url, } diff --git a/src/scheduled_tasks.rs b/src/scheduled_tasks.rs index e591842c6d..2885b5f748 100644 --- a/src/scheduled_tasks.rs +++ b/src/scheduled_tasks.rs @@ -28,7 +28,7 @@ use lemmy_db_schema::{ }, utils::{get_conn, naive_now, now, DbPool, DELETED_REPLACEMENT_TEXT}, }; -use lemmy_routes::nodeinfo::NodeInfo; +use lemmy_routes::nodeinfo::{NodeInfo, NodeInfoWellKnown}; use lemmy_utils::error::LemmyResult; use reqwest_middleware::ClientWithMiddleware; use std::time::Duration; @@ -450,7 +450,10 @@ async fn update_banned_when_expired(pool: &mut DbPool<'_>) { } } -/// Updates the instance software and version +/// Updates the instance software and version. +/// +/// Does so using the /.well-known/nodeinfo protocol described here: +/// https://github.com/jhass/nodeinfo/blob/main/PROTOCOL.md /// /// TODO: if instance has been dead for a long time, it should be checked less frequently async fn update_instance_software( @@ -465,46 +468,7 @@ async fn update_instance_software( let instances = instance::table.get_results::(&mut conn).await?; for instance in instances { - let node_info_url = format!("https://{}/nodeinfo/2.0.json", instance.domain); - - // The `updated` column is used to check if instances are alive. If it is more than three - // days in the past, no outgoing activities will be sent to that instance. However - // not every Fediverse instance has a valid Nodeinfo endpoint (its not required for - // Activitypub). That's why we always need to mark instances as updated if they are - // alive. - let default_form = InstanceForm::builder() - .domain(instance.domain.clone()) - .updated(Some(naive_now())) - .build(); - let form = match client.get(&node_info_url).send().await { - Ok(res) if res.status().is_client_error() => { - // Instance doesn't have nodeinfo but sent a response, consider it alive - Some(default_form) - } - Ok(res) => match res.json::().await { - Ok(node_info) => { - // Instance sent valid nodeinfo, write it to db - let software = node_info.software.as_ref(); - Some( - InstanceForm::builder() - .domain(instance.domain) - .updated(Some(naive_now())) - .software(software.and_then(|s| s.name.clone())) - .version(software.and_then(|s| s.version.clone())) - .build(), - ) - } - Err(_) => { - // No valid nodeinfo but valid HTTP response, consider instance alive - Some(default_form) - } - }, - Err(_) => { - // dead instance, do nothing - None - } - }; - if let Some(form) = form { + if let Some(form) = build_update_instance_form(&instance.domain, client).await { Instance::update(pool, instance.id, form).await?; } } @@ -517,28 +481,107 @@ async fn update_instance_software( Ok(()) } +/// This builds an instance update form, for a given domain. +/// If the instance sends a response, but doesn't have a well-known or nodeinfo, +/// Then return a default form with only the updated field. +async fn build_update_instance_form( + domain: &str, + client: &ClientWithMiddleware, +) -> Option { + // The `updated` column is used to check if instances are alive. If it is more than three + // days in the past, no outgoing activities will be sent to that instance. However + // not every Fediverse instance has a valid Nodeinfo endpoint (its not required for + // Activitypub). That's why we always need to mark instances as updated if they are + // alive. + let mut instance_form = InstanceForm::builder() + .domain(domain.to_string()) + .updated(Some(naive_now())) + .build(); + + // First, fetch their /.well-known/nodeinfo, then extract the correct nodeinfo link from it + let well_known_url = format!("https://{}/.well-known/nodeinfo", domain); + + let Ok(res) = client.get(&well_known_url).send().await else { + // This is the only kind of error that means the instance is dead + return None; + }; + + // In this block, returning `None` is ignored, and only means not writing nodeinfo to db + async { + if res.status().is_client_error() { + return None; + } + + let node_info_url = res + .json::() + .await + .ok()? + .links + .into_iter() + .find(|links| { + links + .rel + .as_str() + .starts_with("http://nodeinfo.diaspora.software/ns/schema/2.") + })? + .href; + + let software = client + .get(node_info_url) + .send() + .await + .ok()? + .json::() + .await + .ok()? + .software?; + + instance_form.software = software.name; + instance_form.version = software.version; + + Some(()) + } + .await; + + Some(instance_form) +} + #[cfg(test)] -#[allow(clippy::unwrap_used)] #[allow(clippy::indexing_slicing)] mod tests { - use lemmy_routes::nodeinfo::NodeInfo; + use crate::scheduled_tasks::build_update_instance_form; + use lemmy_api_common::request::client_builder; + use lemmy_utils::{error::LemmyResult, settings::structs::Settings, LemmyErrorType}; use pretty_assertions::assert_eq; - use reqwest::Client; + use reqwest_middleware::ClientBuilder; + use serial_test::serial; #[tokio::test] - #[ignore] - async fn test_nodeinfo() { - let client = Client::builder().build().unwrap(); - let lemmy_ml_nodeinfo = client - .get("https://lemmy.ml/nodeinfo/2.0.json") - .send() - .await - .unwrap() - .json::() + #[serial] + async fn test_nodeinfo_voyager_lemmy_ml() -> LemmyResult<()> { + let client = ClientBuilder::new(client_builder(&Settings::default()).build()?).build(); + let form = build_update_instance_form("voyager.lemmy.ml", &client) .await - .unwrap(); + .ok_or(LemmyErrorType::CouldntFindObject)?; + assert_eq!( + form.software.ok_or(LemmyErrorType::CouldntFindObject)?, + "lemmy" + ); + Ok(()) + } - assert_eq!(lemmy_ml_nodeinfo.software.unwrap().name.unwrap(), "lemmy"); + #[tokio::test] + #[serial] + async fn test_nodeinfo_mastodon_social() -> LemmyResult<()> { + let client = ClientBuilder::new(client_builder(&Settings::default()).build()?).build(); + let form = build_update_instance_form("mastodon.social", &client) + .await + .ok_or(LemmyErrorType::CouldntFindObject)?; + assert_eq!( + form.software.ok_or(LemmyErrorType::CouldntFindObject)?, + "mastodon" + ); + Ok(()) } }