From 316f858f223162408cfd52183ef7645828c2f480 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Fri, 20 Dec 2024 09:59:14 +0000 Subject: refactor: branch_name handling improve clarity by renaming variables and methods defend against `branch-name` tag with an unsafe name --- src/bin/git_remote_nostr/list.rs | 4 +-- src/bin/git_remote_nostr/push.rs | 6 +++-- src/bin/git_remote_nostr/utils.rs | 4 +-- src/bin/ngit/sub_commands/list.rs | 53 ++++++++++++++++++++++++++------------- 4 files changed, 43 insertions(+), 24 deletions(-) (limited to 'src/bin') diff --git a/src/bin/git_remote_nostr/list.rs b/src/bin/git_remote_nostr/list.rs index 07c6f59..f361272 100644 --- a/src/bin/git_remote_nostr/list.rs +++ b/src/bin/git_remote_nostr/list.rs @@ -149,10 +149,10 @@ async fn get_open_proposals_state( let current_user = get_curent_user(git_repo)?; for (_, (proposal, patches)) in open_proposals { if let Ok(cl) = event_to_cover_letter(&proposal) { - if let Ok(mut branch_name) = cl.get_branch_name() { + if let Ok(mut branch_name) = cl.get_branch_name_with_pr_prefix_and_shorthand_id() { branch_name = if let Some(public_key) = current_user { if proposal.pubkey.eq(&public_key) { - format!("pr/{}", cl.branch_name) + format!("pr/{}", cl.branch_name_without_id_or_prefix) } else { branch_name } diff --git a/src/bin/git_remote_nostr/push.rs b/src/bin/git_remote_nostr/push.rs index 05c8fc2..922808c 100644 --- a/src/bin/git_remote_nostr/push.rs +++ b/src/bin/git_remote_nostr/push.rs @@ -1184,7 +1184,8 @@ async fn create_merge_events( term.write_line( format!( "applied commits from proposal: create nostr proposal status event for {}", - event_to_cover_letter(&proposal)?.get_branch_name()?, + event_to_cover_letter(&proposal)? + .get_branch_name_with_pr_prefix_and_shorthand_id()?, ) .as_str(), )?; @@ -1192,7 +1193,8 @@ async fn create_merge_events( term.write_line( format!( "fast-forward merge: create nostr proposal status event for {}", - event_to_cover_letter(&proposal)?.get_branch_name()?, + event_to_cover_letter(&proposal)? + .get_branch_name_with_pr_prefix_and_shorthand_id()?, ) .as_str(), )?; diff --git a/src/bin/git_remote_nostr/utils.rs b/src/bin/git_remote_nostr/utils.rs index 15ebb10..316fedb 100644 --- a/src/bin/git_remote_nostr/utils.rs +++ b/src/bin/git_remote_nostr/utils.rs @@ -184,10 +184,10 @@ pub async fn get_all_proposals( pub fn find_proposal_and_patches_by_branch_name<'a>( refstr: &'a str, - open_proposals: &'a HashMap)>, + proposals: &'a HashMap)>, current_user: Option<&PublicKey>, ) -> Option<(&'a EventId, &'a (Event, Vec))> { - open_proposals.iter().find(|(_, (proposal, _))| { + proposals.iter().find(|(_, (proposal, _))| { is_event_proposal_root_for_branch(proposal, refstr, current_user).unwrap_or(false) }) } diff --git a/src/bin/ngit/sub_commands/list.rs b/src/bin/ngit/sub_commands/list.rs index 9b84c1b..e8d2e97 100644 --- a/src/bin/ngit/sub_commands/list.rs +++ b/src/bin/ngit/sub_commands/list.rs @@ -261,11 +261,15 @@ pub async fn launch() -> Result<()> { .get_local_branch_names() .context("gitlib2 will not show a list of local branch names")? .iter() - .any(|n| n.eq(&cover_letter.get_branch_name().unwrap())); + .any(|n| { + n.eq(&cover_letter + .get_branch_name_with_pr_prefix_and_shorthand_id() + .unwrap()) + }); let checked_out_proposal_branch = git_repo .get_checked_out_branch_name()? - .eq(&cover_letter.get_branch_name()?); + .eq(&cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?); let proposal_base_commit = str_to_sha1(&tag_value( most_recent_proposal_patch_chain.last().context( @@ -327,14 +331,14 @@ pub async fn launch() -> Result<()> { check_clean(&git_repo)?; let _ = git_repo .apply_patch_chain( - &cover_letter.get_branch_name()?, + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, most_recent_proposal_patch_chain, ) .context("failed to apply patch chain")?; println!( "checked out proposal as '{}' branch", - cover_letter.get_branch_name()? + cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()? ); Ok(()) } @@ -347,7 +351,8 @@ pub async fn launch() -> Result<()> { }; } - let local_branch_tip = git_repo.get_tip_of_branch(&cover_letter.get_branch_name()?)?; + let local_branch_tip = git_repo + .get_tip_of_branch(&cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?)?; // up-to-date if proposal_tip.eq(&local_branch_tip) { @@ -382,10 +387,12 @@ pub async fn launch() -> Result<()> { )? { 0 => { check_clean(&git_repo)?; - git_repo.checkout(&cover_letter.get_branch_name()?)?; + git_repo.checkout( + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, + )?; println!( "checked out proposal as '{}' branch", - cover_letter.get_branch_name()? + cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()? ); Ok(()) } @@ -419,10 +426,12 @@ pub async fn launch() -> Result<()> { )? { 0 => { check_clean(&git_repo)?; - git_repo.checkout(&cover_letter.get_branch_name()?)?; + git_repo.checkout( + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, + )?; let _ = git_repo .apply_patch_chain( - &cover_letter.get_branch_name()?, + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, most_recent_proposal_patch_chain, ) .context("failed to apply patch chain")?; @@ -472,14 +481,16 @@ pub async fn launch() -> Result<()> { 0 => { check_clean(&git_repo)?; git_repo.create_branch_at_commit( - &cover_letter.get_branch_name()?, + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, &proposal_base_commit.to_string(), )?; - git_repo.checkout(&cover_letter.get_branch_name()?)?; + git_repo.checkout( + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, + )?; let chain_length = most_recent_proposal_patch_chain.len(); let _ = git_repo .apply_patch_chain( - &cover_letter.get_branch_name()?, + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, most_recent_proposal_patch_chain, ) .context("failed to apply patch chain")?; @@ -494,7 +505,9 @@ pub async fn launch() -> Result<()> { } 1 => { check_clean(&git_repo)?; - git_repo.checkout(&cover_letter.get_branch_name()?)?; + git_repo.checkout( + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, + )?; println!( "checked out old proposal in existing branch ({} ahead {} behind '{main_branch_name}')", local_ahead_of_main.len(), @@ -537,7 +550,9 @@ pub async fn launch() -> Result<()> { ]), )? { 0 => { - git_repo.checkout(&cover_letter.get_branch_name()?)?; + git_repo.checkout( + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, + )?; println!( "checked out proposal branch with {} unpublished commits ({} ahead {} behind '{main_branch_name}')", local_ahead_of_proposal.len(), @@ -604,7 +619,8 @@ pub async fn launch() -> Result<()> { )? { 0 => { check_clean(&git_repo)?; - git_repo.checkout(&cover_letter.get_branch_name()?)?; + git_repo + .checkout(&cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?)?; println!( "checked out old proposal in existing branch ({} ahead {} behind '{main_branch_name}')", local_ahead_of_main.len(), @@ -615,18 +631,19 @@ pub async fn launch() -> Result<()> { 1 => { check_clean(&git_repo)?; git_repo.create_branch_at_commit( - &cover_letter.get_branch_name()?, + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, &proposal_base_commit.to_string(), )?; let chain_length = most_recent_proposal_patch_chain.len(); let _ = git_repo .apply_patch_chain( - &cover_letter.get_branch_name()?, + &cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?, most_recent_proposal_patch_chain, ) .context("failed to apply patch chain")?; - git_repo.checkout(&cover_letter.get_branch_name()?)?; + git_repo + .checkout(&cover_letter.get_branch_name_with_pr_prefix_and_shorthand_id()?)?; println!( "checked out latest version of proposal ({} ahead {} behind '{main_branch_name}'), replacing unpublished version ({} ahead {} behind '{main_branch_name}')", chain_length, -- cgit v1.2.3