mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 15:08:24 +00:00
Add Git protocol v2 support to fix modern git client compatibility
Modern git clients (2.51.0+) default to protocol v2 and send the Git-Protocol header. The server must pass this to git processes via the GIT_PROTOCOL environment variable for proper negotiation. Changes: - Extract Git-Protocol header in HTTP layer (src/http/mod.rs) - Pass git_protocol parameter through all handler functions - Set GIT_PROTOCOL env var when spawning git subprocesses - Update all tests to pass None for backward compatibility This fixes hangs/timeouts when modern git clients connect to the server. Fixes issue discovered in work/2025-01-07-pr-clone-tag-sync-investigation.md
This commit is contained in:
+7
-3
@@ -26,6 +26,7 @@ use crate::purgatory::Purgatory;
|
||||
pub async fn handle_info_refs(
|
||||
repo_path: PathBuf,
|
||||
service: GitService,
|
||||
git_protocol: Option<&str>,
|
||||
) -> Result<Response<Full<Bytes>>, GitError> {
|
||||
debug!(
|
||||
"Handling info/refs for {:?} with service {:?}",
|
||||
@@ -39,7 +40,7 @@ pub async fn handle_info_refs(
|
||||
}
|
||||
|
||||
// Spawn git with --advertise-refs
|
||||
let mut git = GitSubprocess::spawn(service, &repo_path, true).map_err(|e| {
|
||||
let mut git = GitSubprocess::spawn(service, &repo_path, true, git_protocol).map_err(|e| {
|
||||
error!("Failed to spawn git process: {}", e);
|
||||
GitError::ProcessSpawnFailed(e)
|
||||
})?;
|
||||
@@ -102,6 +103,7 @@ pub async fn handle_info_refs(
|
||||
pub async fn handle_upload_pack(
|
||||
repo_path: PathBuf,
|
||||
request_body: Bytes,
|
||||
git_protocol: Option<&str>,
|
||||
) -> Result<Response<Full<Bytes>>, GitError> {
|
||||
debug!("Handling upload-pack for {:?}", repo_path);
|
||||
|
||||
@@ -110,7 +112,7 @@ pub async fn handle_upload_pack(
|
||||
}
|
||||
|
||||
// Spawn git upload-pack
|
||||
let mut git = GitSubprocess::spawn(GitService::UploadPack, &repo_path, false)
|
||||
let mut git = GitSubprocess::spawn(GitService::UploadPack, &repo_path, false, git_protocol)
|
||||
.map_err(GitError::ProcessSpawnFailed)?;
|
||||
|
||||
// Write request to git's stdin
|
||||
@@ -181,6 +183,7 @@ pub async fn handle_upload_pack(
|
||||
/// * `identifier` - The repository identifier (d tag) for authorization lookup
|
||||
/// * `owner_pubkey` - The owner's public key (hex) from the URL path, scoping authorization
|
||||
/// * `git_data_path` - Base path for git repositories (for syncing to other owner repos)
|
||||
/// * `git_protocol` - Optional Git protocol version (e.g., "version=2")
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub async fn handle_receive_pack(
|
||||
repo_path: PathBuf,
|
||||
@@ -191,6 +194,7 @@ pub async fn handle_receive_pack(
|
||||
owner_pubkey: &str,
|
||||
purgatory: Arc<Purgatory>,
|
||||
git_data_path: &str,
|
||||
git_protocol: Option<&str>,
|
||||
) -> Result<Response<Full<Bytes>>, GitError> {
|
||||
debug!("Handling receive-pack for {:?}", repo_path);
|
||||
|
||||
@@ -236,7 +240,7 @@ pub async fn handle_receive_pack(
|
||||
};
|
||||
|
||||
// Spawn git receive-pack
|
||||
let mut git = GitSubprocess::spawn(GitService::ReceivePack, &repo_path, false)
|
||||
let mut git = GitSubprocess::spawn(GitService::ReceivePack, &repo_path, false, git_protocol)
|
||||
.map_err(GitError::ProcessSpawnFailed)?;
|
||||
|
||||
// Write request to git's stdin
|
||||
|
||||
+10
-2
@@ -22,10 +22,12 @@ impl GitSubprocess {
|
||||
/// * `service` - The Git service (upload-pack or receive-pack)
|
||||
/// * `repo_path` - Path to the bare Git repository
|
||||
/// * `advertise` - If true, run with --advertise-refs flag
|
||||
/// * `git_protocol` - Optional Git protocol version (e.g., "version=2")
|
||||
pub fn spawn(
|
||||
service: GitService,
|
||||
repo_path: impl AsRef<Path>,
|
||||
advertise: bool,
|
||||
git_protocol: Option<&str>,
|
||||
) -> std::io::Result<Self> {
|
||||
let repo_path = repo_path.as_ref();
|
||||
|
||||
@@ -52,6 +54,12 @@ impl GitSubprocess {
|
||||
cmd.stdout(Stdio::piped());
|
||||
cmd.stderr(Stdio::piped());
|
||||
|
||||
// Set GIT_PROTOCOL environment variable if provided
|
||||
// This enables Git protocol v2 support for modern git clients
|
||||
if let Some(protocol) = git_protocol {
|
||||
cmd.env("GIT_PROTOCOL", protocol);
|
||||
}
|
||||
|
||||
let child = cmd.spawn()?;
|
||||
|
||||
Ok(Self { child })
|
||||
@@ -118,7 +126,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn test_spawn_upload_pack_advertise() {
|
||||
let repo = create_bare_repo();
|
||||
let mut proc = GitSubprocess::spawn(GitService::UploadPack, repo.path(), true)
|
||||
let mut proc = GitSubprocess::spawn(GitService::UploadPack, repo.path(), true, None)
|
||||
.expect("Failed to spawn git");
|
||||
|
||||
// Should have spawned successfully
|
||||
@@ -132,7 +140,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn test_spawn_receive_pack() {
|
||||
let repo = create_bare_repo();
|
||||
let mut proc = GitSubprocess::spawn(GitService::ReceivePack, repo.path(), false)
|
||||
let mut proc = GitSubprocess::spawn(GitService::ReceivePack, repo.path(), false, None)
|
||||
.expect("Failed to spawn git");
|
||||
|
||||
assert!(proc.stdout().is_some());
|
||||
|
||||
+23
-4
@@ -152,13 +152,21 @@ impl Service<Request<Incoming>> for HttpService {
|
||||
let identifier = identifier.to_string();
|
||||
let subpath = subpath.to_string();
|
||||
|
||||
// Extract Git-Protocol header for protocol v2 support
|
||||
let git_protocol = req
|
||||
.headers()
|
||||
.get("git-protocol")
|
||||
.and_then(|v| v.to_str().ok())
|
||||
.map(|s| s.to_string());
|
||||
|
||||
tracing::debug!(
|
||||
"Git request: {} {} (npub={}, id={}, subpath={})",
|
||||
"Git request: {} {} (npub={}, id={}, subpath={}, protocol={:?})",
|
||||
method,
|
||||
path,
|
||||
npub,
|
||||
identifier,
|
||||
subpath
|
||||
subpath,
|
||||
git_protocol
|
||||
);
|
||||
|
||||
let repo_path = git::resolve_repo_path(&git_data_path, &npub, &identifier);
|
||||
@@ -185,7 +193,12 @@ impl Service<Request<Incoming>> for HttpService {
|
||||
|
||||
match service {
|
||||
Some(svc) => {
|
||||
let result = git::handlers::handle_info_refs(repo_path, svc).await;
|
||||
let result = git::handlers::handle_info_refs(
|
||||
repo_path,
|
||||
svc,
|
||||
git_protocol.as_deref(),
|
||||
)
|
||||
.await;
|
||||
// Track operation
|
||||
if let Some(ref m) = metrics_clone {
|
||||
let status = if result.is_ok() { "success" } else { "error" };
|
||||
@@ -203,7 +216,12 @@ impl Service<Request<Incoming>> for HttpService {
|
||||
|
||||
// POST /git-upload-pack (clone/fetch)
|
||||
(m, "git-upload-pack") if m == Method::POST => {
|
||||
let result = git::handlers::handle_upload_pack(repo_path, body_bytes).await;
|
||||
let result = git::handlers::handle_upload_pack(
|
||||
repo_path,
|
||||
body_bytes,
|
||||
git_protocol.as_deref(),
|
||||
)
|
||||
.await;
|
||||
if let Some(ref m) = metrics_clone {
|
||||
let status = if result.is_ok() { "success" } else { "error" };
|
||||
m.record_git_operation("clone", status);
|
||||
@@ -238,6 +256,7 @@ impl Service<Request<Incoming>> for HttpService {
|
||||
&owner_pubkey_hex,
|
||||
purgatory.clone(),
|
||||
&git_data_path,
|
||||
git_protocol.as_deref(),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user