diff options
| author | DanConwayDev <DanConwayDev@protonmail.com> | 2026-01-07 21:35:56 +0000 |
|---|---|---|
| committer | DanConwayDev <DanConwayDev@protonmail.com> | 2026-01-07 21:35:56 +0000 |
| commit | 457e296d90e2f7c2808e216f2ef0608b70f76553 (patch) | |
| tree | 8b6529cb9db3bb562842a4cfb0bf8c217ed7db1a /src | |
| parent | 0550b3229f35ef3ee125bac47d85bbd08d1250b1 (diff) | |
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
Diffstat (limited to 'src')
| -rw-r--r-- | src/git/handlers.rs | 10 | ||||
| -rw-r--r-- | src/git/subprocess.rs | 12 | ||||
| -rw-r--r-- | src/http/mod.rs | 27 |
3 files changed, 40 insertions, 9 deletions
diff --git a/src/git/handlers.rs b/src/git/handlers.rs index ff55e34..017eee4 100644 --- a/src/git/handlers.rs +++ b/src/git/handlers.rs | |||
| @@ -26,6 +26,7 @@ use crate::purgatory::Purgatory; | |||
| 26 | pub async fn handle_info_refs( | 26 | pub async fn handle_info_refs( |
| 27 | repo_path: PathBuf, | 27 | repo_path: PathBuf, |
| 28 | service: GitService, | 28 | service: GitService, |
| 29 | git_protocol: Option<&str>, | ||
| 29 | ) -> Result<Response<Full<Bytes>>, GitError> { | 30 | ) -> Result<Response<Full<Bytes>>, GitError> { |
| 30 | debug!( | 31 | debug!( |
| 31 | "Handling info/refs for {:?} with service {:?}", | 32 | "Handling info/refs for {:?} with service {:?}", |
| @@ -39,7 +40,7 @@ pub async fn handle_info_refs( | |||
| 39 | } | 40 | } |
| 40 | 41 | ||
| 41 | // Spawn git with --advertise-refs | 42 | // Spawn git with --advertise-refs |
| 42 | let mut git = GitSubprocess::spawn(service, &repo_path, true).map_err(|e| { | 43 | let mut git = GitSubprocess::spawn(service, &repo_path, true, git_protocol).map_err(|e| { |
| 43 | error!("Failed to spawn git process: {}", e); | 44 | error!("Failed to spawn git process: {}", e); |
| 44 | GitError::ProcessSpawnFailed(e) | 45 | GitError::ProcessSpawnFailed(e) |
| 45 | })?; | 46 | })?; |
| @@ -102,6 +103,7 @@ pub async fn handle_info_refs( | |||
| 102 | pub async fn handle_upload_pack( | 103 | pub async fn handle_upload_pack( |
| 103 | repo_path: PathBuf, | 104 | repo_path: PathBuf, |
| 104 | request_body: Bytes, | 105 | request_body: Bytes, |
| 106 | git_protocol: Option<&str>, | ||
| 105 | ) -> Result<Response<Full<Bytes>>, GitError> { | 107 | ) -> Result<Response<Full<Bytes>>, GitError> { |
| 106 | debug!("Handling upload-pack for {:?}", repo_path); | 108 | debug!("Handling upload-pack for {:?}", repo_path); |
| 107 | 109 | ||
| @@ -110,7 +112,7 @@ pub async fn handle_upload_pack( | |||
| 110 | } | 112 | } |
| 111 | 113 | ||
| 112 | // Spawn git upload-pack | 114 | // Spawn git upload-pack |
| 113 | let mut git = GitSubprocess::spawn(GitService::UploadPack, &repo_path, false) | 115 | let mut git = GitSubprocess::spawn(GitService::UploadPack, &repo_path, false, git_protocol) |
| 114 | .map_err(GitError::ProcessSpawnFailed)?; | 116 | .map_err(GitError::ProcessSpawnFailed)?; |
| 115 | 117 | ||
| 116 | // Write request to git's stdin | 118 | // Write request to git's stdin |
| @@ -181,6 +183,7 @@ pub async fn handle_upload_pack( | |||
| 181 | /// * `identifier` - The repository identifier (d tag) for authorization lookup | 183 | /// * `identifier` - The repository identifier (d tag) for authorization lookup |
| 182 | /// * `owner_pubkey` - The owner's public key (hex) from the URL path, scoping authorization | 184 | /// * `owner_pubkey` - The owner's public key (hex) from the URL path, scoping authorization |
| 183 | /// * `git_data_path` - Base path for git repositories (for syncing to other owner repos) | 185 | /// * `git_data_path` - Base path for git repositories (for syncing to other owner repos) |
| 186 | /// * `git_protocol` - Optional Git protocol version (e.g., "version=2") | ||
| 184 | #[allow(clippy::too_many_arguments)] | 187 | #[allow(clippy::too_many_arguments)] |
| 185 | pub async fn handle_receive_pack( | 188 | pub async fn handle_receive_pack( |
| 186 | repo_path: PathBuf, | 189 | repo_path: PathBuf, |
| @@ -191,6 +194,7 @@ pub async fn handle_receive_pack( | |||
| 191 | owner_pubkey: &str, | 194 | owner_pubkey: &str, |
| 192 | purgatory: Arc<Purgatory>, | 195 | purgatory: Arc<Purgatory>, |
| 193 | git_data_path: &str, | 196 | git_data_path: &str, |
| 197 | git_protocol: Option<&str>, | ||
| 194 | ) -> Result<Response<Full<Bytes>>, GitError> { | 198 | ) -> Result<Response<Full<Bytes>>, GitError> { |
| 195 | debug!("Handling receive-pack for {:?}", repo_path); | 199 | debug!("Handling receive-pack for {:?}", repo_path); |
| 196 | 200 | ||
| @@ -236,7 +240,7 @@ pub async fn handle_receive_pack( | |||
| 236 | }; | 240 | }; |
| 237 | 241 | ||
| 238 | // Spawn git receive-pack | 242 | // Spawn git receive-pack |
| 239 | let mut git = GitSubprocess::spawn(GitService::ReceivePack, &repo_path, false) | 243 | let mut git = GitSubprocess::spawn(GitService::ReceivePack, &repo_path, false, git_protocol) |
| 240 | .map_err(GitError::ProcessSpawnFailed)?; | 244 | .map_err(GitError::ProcessSpawnFailed)?; |
| 241 | 245 | ||
| 242 | // Write request to git's stdin | 246 | // Write request to git's stdin |
diff --git a/src/git/subprocess.rs b/src/git/subprocess.rs index 2d9a981..acee726 100644 --- a/src/git/subprocess.rs +++ b/src/git/subprocess.rs | |||
| @@ -22,10 +22,12 @@ impl GitSubprocess { | |||
| 22 | /// * `service` - The Git service (upload-pack or receive-pack) | 22 | /// * `service` - The Git service (upload-pack or receive-pack) |
| 23 | /// * `repo_path` - Path to the bare Git repository | 23 | /// * `repo_path` - Path to the bare Git repository |
| 24 | /// * `advertise` - If true, run with --advertise-refs flag | 24 | /// * `advertise` - If true, run with --advertise-refs flag |
| 25 | /// * `git_protocol` - Optional Git protocol version (e.g., "version=2") | ||
| 25 | pub fn spawn( | 26 | pub fn spawn( |
| 26 | service: GitService, | 27 | service: GitService, |
| 27 | repo_path: impl AsRef<Path>, | 28 | repo_path: impl AsRef<Path>, |
| 28 | advertise: bool, | 29 | advertise: bool, |
| 30 | git_protocol: Option<&str>, | ||
| 29 | ) -> std::io::Result<Self> { | 31 | ) -> std::io::Result<Self> { |
| 30 | let repo_path = repo_path.as_ref(); | 32 | let repo_path = repo_path.as_ref(); |
| 31 | 33 | ||
| @@ -52,6 +54,12 @@ impl GitSubprocess { | |||
| 52 | cmd.stdout(Stdio::piped()); | 54 | cmd.stdout(Stdio::piped()); |
| 53 | cmd.stderr(Stdio::piped()); | 55 | cmd.stderr(Stdio::piped()); |
| 54 | 56 | ||
| 57 | // Set GIT_PROTOCOL environment variable if provided | ||
| 58 | // This enables Git protocol v2 support for modern git clients | ||
| 59 | if let Some(protocol) = git_protocol { | ||
| 60 | cmd.env("GIT_PROTOCOL", protocol); | ||
| 61 | } | ||
| 62 | |||
| 55 | let child = cmd.spawn()?; | 63 | let child = cmd.spawn()?; |
| 56 | 64 | ||
| 57 | Ok(Self { child }) | 65 | Ok(Self { child }) |
| @@ -118,7 +126,7 @@ mod tests { | |||
| 118 | #[tokio::test] | 126 | #[tokio::test] |
| 119 | async fn test_spawn_upload_pack_advertise() { | 127 | async fn test_spawn_upload_pack_advertise() { |
| 120 | let repo = create_bare_repo(); | 128 | let repo = create_bare_repo(); |
| 121 | let mut proc = GitSubprocess::spawn(GitService::UploadPack, repo.path(), true) | 129 | let mut proc = GitSubprocess::spawn(GitService::UploadPack, repo.path(), true, None) |
| 122 | .expect("Failed to spawn git"); | 130 | .expect("Failed to spawn git"); |
| 123 | 131 | ||
| 124 | // Should have spawned successfully | 132 | // Should have spawned successfully |
| @@ -132,7 +140,7 @@ mod tests { | |||
| 132 | #[tokio::test] | 140 | #[tokio::test] |
| 133 | async fn test_spawn_receive_pack() { | 141 | async fn test_spawn_receive_pack() { |
| 134 | let repo = create_bare_repo(); | 142 | let repo = create_bare_repo(); |
| 135 | let mut proc = GitSubprocess::spawn(GitService::ReceivePack, repo.path(), false) | 143 | let mut proc = GitSubprocess::spawn(GitService::ReceivePack, repo.path(), false, None) |
| 136 | .expect("Failed to spawn git"); | 144 | .expect("Failed to spawn git"); |
| 137 | 145 | ||
| 138 | assert!(proc.stdout().is_some()); | 146 | assert!(proc.stdout().is_some()); |
diff --git a/src/http/mod.rs b/src/http/mod.rs index e2caf5d..9172e86 100644 --- a/src/http/mod.rs +++ b/src/http/mod.rs | |||
| @@ -152,13 +152,21 @@ impl Service<Request<Incoming>> for HttpService { | |||
| 152 | let identifier = identifier.to_string(); | 152 | let identifier = identifier.to_string(); |
| 153 | let subpath = subpath.to_string(); | 153 | let subpath = subpath.to_string(); |
| 154 | 154 | ||
| 155 | // Extract Git-Protocol header for protocol v2 support | ||
| 156 | let git_protocol = req | ||
| 157 | .headers() | ||
| 158 | .get("git-protocol") | ||
| 159 | .and_then(|v| v.to_str().ok()) | ||
| 160 | .map(|s| s.to_string()); | ||
| 161 | |||
| 155 | tracing::debug!( | 162 | tracing::debug!( |
| 156 | "Git request: {} {} (npub={}, id={}, subpath={})", | 163 | "Git request: {} {} (npub={}, id={}, subpath={}, protocol={:?})", |
| 157 | method, | 164 | method, |
| 158 | path, | 165 | path, |
| 159 | npub, | 166 | npub, |
| 160 | identifier, | 167 | identifier, |
| 161 | subpath | 168 | subpath, |
| 169 | git_protocol | ||
| 162 | ); | 170 | ); |
| 163 | 171 | ||
| 164 | let repo_path = git::resolve_repo_path(&git_data_path, &npub, &identifier); | 172 | let repo_path = git::resolve_repo_path(&git_data_path, &npub, &identifier); |
| @@ -185,7 +193,12 @@ impl Service<Request<Incoming>> for HttpService { | |||
| 185 | 193 | ||
| 186 | match service { | 194 | match service { |
| 187 | Some(svc) => { | 195 | Some(svc) => { |
| 188 | let result = git::handlers::handle_info_refs(repo_path, svc).await; | 196 | let result = git::handlers::handle_info_refs( |
| 197 | repo_path, | ||
| 198 | svc, | ||
| 199 | git_protocol.as_deref(), | ||
| 200 | ) | ||
| 201 | .await; | ||
| 189 | // Track operation | 202 | // Track operation |
| 190 | if let Some(ref m) = metrics_clone { | 203 | if let Some(ref m) = metrics_clone { |
| 191 | let status = if result.is_ok() { "success" } else { "error" }; | 204 | let status = if result.is_ok() { "success" } else { "error" }; |
| @@ -203,7 +216,12 @@ impl Service<Request<Incoming>> for HttpService { | |||
| 203 | 216 | ||
| 204 | // POST /git-upload-pack (clone/fetch) | 217 | // POST /git-upload-pack (clone/fetch) |
| 205 | (m, "git-upload-pack") if m == Method::POST => { | 218 | (m, "git-upload-pack") if m == Method::POST => { |
| 206 | let result = git::handlers::handle_upload_pack(repo_path, body_bytes).await; | 219 | let result = git::handlers::handle_upload_pack( |
| 220 | repo_path, | ||
| 221 | body_bytes, | ||
| 222 | git_protocol.as_deref(), | ||
| 223 | ) | ||
| 224 | .await; | ||
| 207 | if let Some(ref m) = metrics_clone { | 225 | if let Some(ref m) = metrics_clone { |
| 208 | let status = if result.is_ok() { "success" } else { "error" }; | 226 | let status = if result.is_ok() { "success" } else { "error" }; |
| 209 | m.record_git_operation("clone", status); | 227 | m.record_git_operation("clone", status); |
| @@ -238,6 +256,7 @@ impl Service<Request<Incoming>> for HttpService { | |||
| 238 | &owner_pubkey_hex, | 256 | &owner_pubkey_hex, |
| 239 | purgatory.clone(), | 257 | purgatory.clone(), |
| 240 | &git_data_path, | 258 | &git_data_path, |
| 259 | git_protocol.as_deref(), | ||
| 241 | ) | 260 | ) |
| 242 | .await; | 261 | .await; |
| 243 | 262 | ||