diff options
| author | DanConwayDev <DanConwayDev@protonmail.com> | 2026-04-22 13:00:17 +0000 |
|---|---|---|
| committer | DanConwayDev <DanConwayDev@protonmail.com> | 2026-04-22 13:00:17 +0000 |
| commit | e98acf8c5da5033b20aee8510485a8b4a4f4a56e (patch) | |
| tree | e513e9edb9feca3d9c079026002fbe9cd3d0cec9 /src/bin/git_remote_nostr/fetch.rs | |
| parent | 205ca05897cbc727d9b75e7ab68375b5c93ead39 (diff) | |
fix: prevent fatal clone/fetch errors when PR git data unavailable
Only advertise `refs/heads/pr/*` branches once their tip OIDs are
confirmed present locally; prevents `fatal: bad object` / `remote did
not send all necessary objects` errors during clone/fetch when a PR tip
lives on a different git server than the one that won the bulk prefetch
race.
After the bulk prefetch, collect remaining missing PR tip OIDs and do
one batch fetch per repo git server using only the OIDs that server has
advertised; break early once all are satisfied and skip servers that
carry none. Avoids batch-poisoning (a server rejects the whole request
if any single OID is absent) and redundant connections.
Restrict mop-up fetches and run_fetch to the repo's declared git
servers; do not fetch from clone-tag URLs in PR events - they are
submitter-supplied and could let a malicious or slow server stall every
clone/fetch operation.
Also apply rustfmt and fix clippy warnings.
Diffstat (limited to 'src/bin/git_remote_nostr/fetch.rs')
| -rw-r--r-- | src/bin/git_remote_nostr/fetch.rs | 65 |
1 files changed, 16 insertions, 49 deletions
diff --git a/src/bin/git_remote_nostr/fetch.rs b/src/bin/git_remote_nostr/fetch.rs index dd80f0f..7847777 100644 --- a/src/bin/git_remote_nostr/fetch.rs +++ b/src/bin/git_remote_nostr/fetch.rs | |||
| @@ -1,17 +1,11 @@ | |||
| 1 | use core::str; | 1 | use core::str; |
| 2 | use std::{ | 2 | use std::{collections::HashMap, io::Stdin}; |
| 3 | collections::{HashMap, HashSet}, | ||
| 4 | io::Stdin, | ||
| 5 | }; | ||
| 6 | 3 | ||
| 7 | use anyhow::{Context, Result, bail}; | 4 | use anyhow::{Context, Result, bail}; |
| 8 | use ngit::{ | 5 | use ngit::{ |
| 9 | fetch::fetch_from_git_server, | 6 | fetch::fetch_from_git_server, |
| 10 | git::{Repo, RepoActions}, | 7 | git::{Repo, RepoActions}, |
| 11 | git_events::{ | 8 | git_events::{KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE}, |
| 12 | KIND_PULL_REQUEST, KIND_PULL_REQUEST_UPDATE, | ||
| 13 | identify_clone_urls_for_oids_from_pr_pr_update_events, | ||
| 14 | }, | ||
| 15 | login::get_curent_user, | 9 | login::get_curent_user, |
| 16 | repo_ref::{RepoRef, is_grasp_server_in_list}, | 10 | repo_ref::{RepoRef, is_grasp_server_in_list}, |
| 17 | utils::{ | 11 | utils::{ |
| @@ -37,56 +31,29 @@ pub async fn run_fetch( | |||
| 37 | .map(|(_, oid)| oid.clone()) | 31 | .map(|(_, oid)| oid.clone()) |
| 38 | .collect::<Vec<String>>(); | 32 | .collect::<Vec<String>>(); |
| 39 | 33 | ||
| 40 | let pr_oid_clone_url_map = identify_clone_urls_for_oids_from_pr_pr_update_events( | ||
| 41 | fetch_batch.values().collect::<Vec<&String>>(), | ||
| 42 | git_repo, | ||
| 43 | repo_ref, | ||
| 44 | ) | ||
| 45 | .await?; | ||
| 46 | |||
| 47 | let oids_to_fetch_from_git_servers = [ | ||
| 48 | oids_from_state.clone(), | ||
| 49 | pr_oid_clone_url_map | ||
| 50 | .keys() | ||
| 51 | .cloned() | ||
| 52 | .collect::<Vec<String>>(), | ||
| 53 | ] | ||
| 54 | .concat(); | ||
| 55 | |||
| 56 | let git_servers = { | ||
| 57 | let mut seen: HashSet<String> = HashSet::new(); | ||
| 58 | let mut out: Vec<String> = vec![]; | ||
| 59 | for server in &repo_ref.git_server { | ||
| 60 | if seen.insert(server.clone()) { | ||
| 61 | out.push(server.clone()); | ||
| 62 | } | ||
| 63 | } | ||
| 64 | for url in pr_oid_clone_url_map.values().flatten() { | ||
| 65 | if seen.insert(url.clone()) { | ||
| 66 | out.push(url.clone()); | ||
| 67 | } | ||
| 68 | } | ||
| 69 | out | ||
| 70 | }; | ||
| 71 | |||
| 72 | let mut errors = vec![]; | 34 | let mut errors = vec![]; |
| 73 | let term = console::Term::stderr(); | 35 | let term = console::Term::stderr(); |
| 74 | 36 | ||
| 75 | for git_server_url in &git_servers { | 37 | // Only repo git servers are tried here. PR tip OIDs are guaranteed local by the |
| 76 | let oids_to_fetch_from_server = oids_to_fetch_from_git_servers | 38 | // list phase (see get_open_and_draft_proposals_state), so run_fetch only needs |
| 77 | .clone() | 39 | // to resolve state OIDs that may have been missed by the bulk prefetch. |
| 78 | .into_iter() | 40 | // We intentionally do not fall back to git-server URLs in PR event `clone` |
| 41 | // tags; see the note in get_open_and_draft_proposals_state for the | ||
| 42 | // rationale. | ||
| 43 | for git_server_url in &repo_ref.git_server { | ||
| 44 | let missing = oids_from_state | ||
| 45 | .iter() | ||
| 79 | .filter(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false)) | 46 | .filter(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false)) |
| 47 | .cloned() | ||
| 80 | .collect::<Vec<String>>(); | 48 | .collect::<Vec<String>>(); |
| 81 | 49 | ||
| 82 | if oids_to_fetch_from_server.is_empty() { | 50 | if missing.is_empty() { |
| 83 | continue; | 51 | break; |
| 84 | } | 52 | } |
| 85 | 53 | ||
| 86 | let term = console::Term::stderr(); | ||
| 87 | if let Err(error) = fetch_from_git_server( | 54 | if let Err(error) = fetch_from_git_server( |
| 88 | git_repo, | 55 | git_repo, |
| 89 | &oids_from_state, | 56 | &missing, |
| 90 | git_server_url, | 57 | git_server_url, |
| 91 | &repo_ref.to_nostr_git_url(&None), | 58 | &repo_ref.to_nostr_git_url(&None), |
| 92 | &term, | 59 | &term, |
| @@ -98,7 +65,7 @@ pub async fn run_fetch( | |||
| 98 | 65 | ||
| 99 | if oids_from_state | 66 | if oids_from_state |
| 100 | .iter() | 67 | .iter() |
| 101 | .any(|oid| !git_repo.does_commit_exist(oid).unwrap()) | 68 | .any(|oid| !git_repo.does_commit_exist(oid).unwrap_or(false)) |
| 102 | && !errors.is_empty() | 69 | && !errors.is_empty() |
| 103 | { | 70 | { |
| 104 | bail!( | 71 | bail!( |