Adding a vector indexing check to prevent panics. Fixes #2753 (#2754)

* Adding a vector indexing check to prevent panics. Fixes #2753

* Fixing for new clippy lint.

* Externalizing get_top_mod check. Adding get_first clippy lint

* Removing unit tests get(0)s

* Fixing some firsts manually

.drone.yml

@@ -68,7 +68,10 @@ steps:
           -D clippy::manual_string_new -D clippy::redundant_closure_for_method_calls 
           -D clippy::unused_self
           -A clippy::uninlined_format_args
-      - cargo clippy --workspace --features console -- -D clippy::unwrap_used
+          -D clippy::get_first
+      - cargo clippy --workspace --features console -- 
+        -D clippy::unwrap_used
+        -D clippy::indexing_slicing
 
   - name: lemmy_api_common doesnt depend on diesel
     image: clux/muslrust:1.67.0

crates/api/src/community/transfer.rs

@@ -4,7 +4,7 @@ use anyhow::Context;
 use lemmy_api_common::{
   community::{GetCommunityResponse, TransferCommunity},
   context::LemmyContext,
-  utils::get_local_user_view_from_jwt,
+  utils::{get_local_user_view_from_jwt, is_admin, is_top_mod},
 };
 use lemmy_db_schema::{
   source::{
@@ -13,7 +13,7 @@ use lemmy_db_schema::{
   },
   traits::{Crud, Joinable},
 };
-use lemmy_db_views_actor::structs::{CommunityModeratorView, CommunityView, PersonViewSafe};
+use lemmy_db_views_actor::structs::{CommunityModeratorView, CommunityView};
 use lemmy_utils::{error::LemmyError, location_info, ConnectionId};
 
 // TODO: we dont do anything for federation here, it should be updated the next time the community
@@ -32,19 +32,14 @@ impl Perform for TransferCommunity {
     let local_user_view =
       get_local_user_view_from_jwt(&data.auth, context.pool(), context.secret()).await?;
 
-    let admins = PersonViewSafe::admins(context.pool()).await?;
-
     // Fetch the community mods
     let community_id = data.community_id;
     let mut community_mods =
       CommunityModeratorView::for_community(context.pool(), community_id).await?;
 
     // Make sure transferrer is either the top community mod, or an admin
-    if local_user_view.person.id != community_mods[0].moderator.id
-      && !admins
-        .iter()
-        .map(|a| a.person.id)
-        .any(|x| x == local_user_view.person.id)
+    if !(is_top_mod(&local_user_view, &community_mods).is_ok()
+      || is_admin(&local_user_view).is_ok())
     {
       return Err(LemmyError::from_message("not_an_admin"));
     }

crates/api_common/src/request.rs

@@ -81,7 +81,7 @@ fn html_to_site_metadata(html_bytes: &[u8]) -> Result<SiteMetadata, LemmyError>
   let og_image = page
     .opengraph
     .images
-    .get(0)
+    .first()
     .and_then(|ogo| Url::parse(&ogo.url).ok());
   let og_embed_url = page
     .opengraph
@@ -200,6 +200,9 @@ pub async fn fetch_site_data(
       // Warning, this may ignore SSL errors
       let metadata_option = fetch_site_metadata(client, url).await.ok();
 
+      let missing_pictrs_file =
+        |r: PictrsResponse| r.files.first().expect("missing pictrs file").file.clone();
+
       // Fetch pictrs thumbnail
       let pictrs_hash = match &metadata_option {
         Some(metadata_res) => match &metadata_res.image {
@@ -207,16 +210,16 @@ pub async fn fetch_site_data(
           // Try to generate a small thumbnail if there's a full sized one from post-links
           Some(metadata_image) => fetch_pictrs(client, settings, metadata_image)
             .await
-            .map(|r| r.files[0].file.clone()),
+            .map(missing_pictrs_file),
           // Metadata, but no image
           None => fetch_pictrs(client, settings, url)
             .await
-            .map(|r| r.files[0].file.clone()),
+            .map(missing_pictrs_file),
         },
         // No metadata, try to fetch the URL as an image
         None => fetch_pictrs(client, settings, url)
           .await
-          .map(|r| r.files[0].file.clone()),
+          .map(missing_pictrs_file),
       };
 
       // The full urls are necessary for federation

crates/api_common/src/utils.rs

@@ -64,7 +64,7 @@ pub async fn is_mod_or_admin(
 pub async fn is_top_admin(pool: &DbPool, person_id: PersonId) -> Result<(), LemmyError> {
   let admins = PersonViewSafe::admins(pool).await?;
   let top_admin = admins
-    .get(0)
+    .first()
     .ok_or_else(|| LemmyError::from_message("no admins"))?;
 
   if top_admin.person.id != person_id {
@@ -80,6 +80,21 @@ pub fn is_admin(local_user_view: &LocalUserView) -> Result<(), LemmyError> {
   Ok(())
 }
 
+pub fn is_top_mod(
+  local_user_view: &LocalUserView,
+  community_mods: &[CommunityModeratorView],
+) -> Result<(), LemmyError> {
+  if local_user_view.person.id
+    != community_mods
+      .first()
+      .map(|cm| cm.moderator.id)
+      .unwrap_or(PersonId(0))
+  {
+    return Err(LemmyError::from_message("not_top_mod"));
+  }
+  Ok(())
+}
+
 #[tracing::instrument(skip_all)]
 pub async fn get_post(post_id: PostId, pool: &DbPool) -> Result<Post, LemmyError> {
   Post::read(pool, post_id)

crates/api_crud/src/community/delete.rs

@@ -3,7 +3,7 @@ use actix_web::web::Data;
 use lemmy_api_common::{
   community::{CommunityResponse, DeleteCommunity},
   context::LemmyContext,
-  utils::get_local_user_view_from_jwt,
+  utils::{get_local_user_view_from_jwt, is_top_mod},
   websocket::{send::send_community_ws_message, UserOperationCrud},
 };
 use lemmy_db_schema::{
@@ -33,9 +33,7 @@ impl PerformCrud for DeleteCommunity {
       CommunityModeratorView::for_community(context.pool(), community_id).await?;
 
     // Make sure deleter is the top mod
-    if local_user_view.person.id != community_mods[0].moderator.id {
-      return Err(LemmyError::from_message("no_community_edit_allowed"));
-    }
+    is_top_mod(&local_user_view, &community_mods)?;
 
     // Do the delete
     let community_id = data.community_id;

crates/apub/src/collections/community_featured.rs

@@ -76,7 +76,10 @@ impl ApubObject for ApubCommunityFeatured {
   {
     let mut posts = apub.ordered_items;
     if posts.len() as i64 > FETCH_LIMIT_MAX {
-      posts = posts[0..(FETCH_LIMIT_MAX as usize)].to_vec();
+      posts = posts
+        .get(0..(FETCH_LIMIT_MAX as usize))
+        .unwrap_or_default()
+        .to_vec();
     }
 
     // We intentionally ignore errors here. This is because the outbox might contain posts from old

crates/apub/src/collections/community_outbox.rs

@@ -101,7 +101,10 @@ impl ApubObject for ApubCommunityOutbox {
   ) -> Result<Self, LemmyError> {
     let mut outbox_activities = apub.ordered_items;
     if outbox_activities.len() as i64 > FETCH_LIMIT_MAX {
-      outbox_activities = outbox_activities[0..(FETCH_LIMIT_MAX as usize)].to_vec();
+      outbox_activities = outbox_activities
+        .get(0..(FETCH_LIMIT_MAX as usize))
+        .unwrap_or_default()
+        .to_vec();
     }
 
     // We intentionally ignore errors here. This is because the outbox might contain posts from old

crates/db_schema/src/impls/actor_language.rs

@@ -288,8 +288,8 @@ pub async fn default_post_language(
     .get_results::<LanguageId>(conn)
     .await?;
 
-  if intersection.len() == 1 {
-    Ok(Some(intersection[0]))
+  if let Some(i) = intersection.get(0) {
+    Ok(Some(*i))
   } else {
     Ok(None)
   }

crates/db_schema/src/impls/comment.rs

@@ -98,9 +98,13 @@ impl Comment {
         // left join comment c2 on c2.path <@ c.path and c2.path != c.path
         // group by c.id
 
-        let top_parent = format!("0.{}", parent_path.0.split('.').collect::<Vec<&str>>()[1]);
-        let update_child_count_stmt = format!(
-          "
+        let path_split = parent_path.0.split('.').collect::<Vec<&str>>();
+        let parent_id = path_split.get(1);
+
+        if let Some(parent_id) = parent_id {
+          let top_parent = format!("0.{}", parent_id);
+          let update_child_count_stmt = format!(
+            "
 update comment_aggregates ca set child_count = c.child_count
 from (
   select c.id, c.path, count(c2.id) as child_count from comment c
@@ -109,9 +113,10 @@ from (
   group by c.id
 ) as c
 where ca.comment_id = c.id"
-        );
+          );
 
-        sql_query(update_child_count_stmt).execute(conn).await?;
+          sql_query(update_child_count_stmt).execute(conn).await?;
+        }
       }
       updated_comment
     } else {
@@ -135,10 +140,8 @@ where ca.comment_id = c.id"
     let mut ltree_split: Vec<&str> = self.path.0.split('.').collect();
     ltree_split.remove(0); // The first is always 0
     if ltree_split.len() > 1 {
-      ltree_split[ltree_split.len() - 2]
-        .parse::<i32>()
-        .map(CommentId)
-        .ok()
+      let parent_comment_id = ltree_split.get(ltree_split.len() - 2);
+      parent_comment_id.and_then(|p| p.parse::<i32>().map(CommentId).ok())
     } else {
       None
     }

crates/utils/src/email.rs

@@ -30,18 +30,17 @@ pub fn send_email(
 
   let (smtp_server, smtp_port) = {
     let email_and_port = email_config.smtp_server.split(':').collect::<Vec<&str>>();
-    if email_and_port.len() == 1 {
-      return Err(LemmyError::from_message(
-        "email.smtp_server needs a port, IE smtp.xxx.com:465",
-      ));
-    }
+    let email = *email_and_port
+      .first()
+      .ok_or_else(|| LemmyError::from_message("missing an email"))?;
+    let port = email_and_port
+      .get(1)
+      .ok_or_else(|| {
+        LemmyError::from_message("email.smtp_server needs a port, IE smtp.xxx.com:465")
+      })?
+      .parse::<u16>()?;
 
-    (
-      email_and_port[0],
-      email_and_port[1]
-        .parse::<u16>()
-        .expect("email needs a port"),
-    )
+    (email, port)
   };
 
   // the message length before wrap, 78, is somewhat arbritary but looks good to me

crates/utils/src/utils/mention.rs

@@ -24,10 +24,11 @@ impl MentionData {
 pub fn scrape_text_for_mentions(text: &str) -> Vec<MentionData> {
   let mut out: Vec<MentionData> = Vec::new();
   for caps in MENTIONS_REGEX.captures_iter(text) {
-    out.push(MentionData {
-      name: caps["name"].to_string(),
-      domain: caps["domain"].to_string(),
-    });
+    if let Some(name) = caps.name("name").map(|c| c.as_str().to_string()) {
+      if let Some(domain) = caps.name("domain").map(|c| c.as_str().to_string()) {
+        out.push(MentionData { name, domain });
+      }
+    }
   }
   out.into_iter().unique().collect()
 }

scripts/fix-clippy.sh

@@ -13,6 +13,11 @@ cargo clippy --workspace --fix --allow-staged --allow-dirty --tests --all-target
   -D clippy::wildcard_imports -D clippy::cast_lossless \
   -D clippy::manual_string_new -D clippy::redundant_closure_for_method_calls \
   -D clippy::unused_self \
-  -A clippy::uninlined_format_args
+  -A clippy::uninlined_format_args \
+  -D clippy::get_first
+
+cargo clippy --workspace --features console -- \
+  -D clippy::unwrap_used \
+  -D clippy::indexing_slicing
 
 cargo +nightly fmt

src/api_routes_websocket.rs

@@ -248,10 +248,14 @@ async fn parse_json_message(
   context: LemmyContext,
 ) -> Result<String, LemmyError> {
   let json: Value = serde_json::from_str(&msg)?;
-  let data = &json["data"].to_string();
-  let op = &json["op"]
-    .as_str()
-    .ok_or_else(|| LemmyError::from_message("missing op"))?;
+  let data = &json
+    .get("data")
+    .ok_or_else(|| LemmyError::from_message("missing data"))?
+    .to_string();
+  let op = &json
+    .get("op")
+    .ok_or_else(|| LemmyError::from_message("missing op"))?
+    .to_string();
 
   // check if api call passes the rate limit, and generate future for later execution
   if let Ok(user_operation_crud) = UserOperationCrud::from_str(op) {

src/lib.rs

@@ -47,7 +47,7 @@ pub(crate) const REQWEST_TIMEOUT: Duration = Duration::from_secs(10);
 /// Placing the main function in lib.rs allows other crates to import it and embed Lemmy
 pub async fn start_lemmy_server() -> Result<(), LemmyError> {
   let args: Vec<String> = env::args().collect();
-  if args.len() == 2 && args[1] == "--print-config-docs" {
+  if args.get(1) == Some(&"--print-config-docs".to_string()) {
     let fmt = Formatting {
       auto_comments: AutoComments::none(),
       comments_style: CommentsStyle {