diff options
| author | DanConwayDev <DanConwayDev@protonmail.com> | 2026-01-27 19:43:41 +0000 |
|---|---|---|
| committer | DanConwayDev <DanConwayDev@protonmail.com> | 2026-01-27 20:38:25 +0000 |
| commit | 3a7fa1d1288c28eae0ee58b4c448c672ec3b69c2 (patch) | |
| tree | 13afc5d8abeb0a358701ede483d6ec4a7485c9f4 /src | |
| parent | 10eab82164bb91236f2afa6b7919d0710609ba7f (diff) | |
fix: return HTTP 200 with ERR pkt-line for git protocol errors
Previously, all git upload-pack/receive-pack failures returned HTTP 500,
but the git smart HTTP protocol requires protocol-level errors (like
"not our ref") to be returned as HTTP 200 OK with an ERR pkt-line in
the response body.
Changes:
- Add build_git_protocol_error_response() to create HTTP 200 responses
with properly formatted ERR pkt-line ("ERR <message>\n")
- Add is_git_protocol_error() to detect protocol errors (exit code 128
with stderr content) vs transport errors
- Update handle_upload_pack() and handle_receive_pack() to return
protocol errors as HTTP 200 with ERR pkt-line
- Keep HTTP 500 for actual transport errors (spawn failures, I/O errors,
signals)
This allows git clients to properly parse and display protocol error
messages instead of seeing generic HTTP 500 errors.
Diffstat (limited to 'src')
| -rw-r--r-- | src/git/handlers.rs | 66 |
1 files changed, 66 insertions, 0 deletions
diff --git a/src/git/handlers.rs b/src/git/handlers.rs index 017eee4..e3a6ad4 100644 --- a/src/git/handlers.rs +++ b/src/git/handlers.rs | |||
| @@ -99,6 +99,42 @@ pub async fn handle_info_refs( | |||
| 99 | .unwrap()) | 99 | .unwrap()) |
| 100 | } | 100 | } |
| 101 | 101 | ||
| 102 | /// Build an HTTP 200 OK response with an ERR pkt-line for git protocol errors. | ||
| 103 | /// | ||
| 104 | /// Per the git smart HTTP protocol spec, protocol-level errors (like "not our ref") | ||
| 105 | /// should be returned as HTTP 200 OK with the error message in pkt-line format: | ||
| 106 | /// `PKT-LINE("ERR" SP explanation-text)` | ||
| 107 | /// | ||
| 108 | /// This allows git clients to properly parse and display the error message. | ||
| 109 | fn build_git_protocol_error_response( | ||
| 110 | service: GitService, | ||
| 111 | error_message: &str, | ||
| 112 | ) -> Response<Full<Bytes>> { | ||
| 113 | // Format: "ERR <message>\n" | ||
| 114 | let err_content = format!("ERR {}\n", error_message.trim()); | ||
| 115 | let err_pktline = PktLine::data(err_content.as_bytes()).encode(); | ||
| 116 | |||
| 117 | Response::builder() | ||
| 118 | .status(StatusCode::OK) | ||
| 119 | .header("content-type", service.result_content_type()) | ||
| 120 | .header("cache-control", "no-cache") | ||
| 121 | .body(Full::new(Bytes::from(err_pktline))) | ||
| 122 | .unwrap() | ||
| 123 | } | ||
| 124 | |||
| 125 | /// Check if a git process failure is a protocol error (vs transport error). | ||
| 126 | /// | ||
| 127 | /// Protocol errors are communicated via stderr when git exits with code 128. | ||
| 128 | /// These should be returned to the client as HTTP 200 with ERR pkt-line. | ||
| 129 | /// | ||
| 130 | /// Transport errors (process spawn failures, I/O errors, signals) should | ||
| 131 | /// remain as HTTP 500 errors. | ||
| 132 | fn is_git_protocol_error(exit_code: Option<i32>, stderr: &[u8]) -> bool { | ||
| 133 | // Git uses exit code 128 for protocol/usage errors | ||
| 134 | // If there's stderr content, it's a protocol error message | ||
| 135 | exit_code == Some(128) && !stderr.is_empty() | ||
| 136 | } | ||
| 137 | |||
| 102 | /// Handle POST /git-upload-pack (clone/fetch) | 138 | /// Handle POST /git-upload-pack (clone/fetch) |
| 103 | pub async fn handle_upload_pack( | 139 | pub async fn handle_upload_pack( |
| 104 | repo_path: PathBuf, | 140 | repo_path: PathBuf, |
| @@ -150,6 +186,21 @@ pub async fn handle_upload_pack( | |||
| 150 | 186 | ||
| 151 | if !status.success() { | 187 | if !status.success() { |
| 152 | let stderr_str = String::from_utf8_lossy(&stderr_output); | 188 | let stderr_str = String::from_utf8_lossy(&stderr_output); |
| 189 | |||
| 190 | // Check if this is a git protocol error (exit code 128 with stderr) | ||
| 191 | // Protocol errors should be returned as HTTP 200 with ERR pkt-line | ||
| 192 | if is_git_protocol_error(status.code(), &stderr_output) { | ||
| 193 | warn!( | ||
| 194 | "Git upload-pack protocol error (returning ERR pkt-line): {}", | ||
| 195 | stderr_str | ||
| 196 | ); | ||
| 197 | return Ok(build_git_protocol_error_response( | ||
| 198 | GitService::UploadPack, | ||
| 199 | &stderr_str, | ||
| 200 | )); | ||
| 201 | } | ||
| 202 | |||
| 203 | // Transport errors (spawn failures, signals, etc.) remain as HTTP 500 | ||
| 153 | error!("Git upload-pack failed: {}", stderr_str); | 204 | error!("Git upload-pack failed: {}", stderr_str); |
| 154 | return Err(GitError::GitFailed(status.code())); | 205 | return Err(GitError::GitFailed(status.code())); |
| 155 | } | 206 | } |
| @@ -277,6 +328,21 @@ pub async fn handle_receive_pack( | |||
| 277 | 328 | ||
| 278 | if !status.success() { | 329 | if !status.success() { |
| 279 | let stderr_str = String::from_utf8_lossy(&stderr_output); | 330 | let stderr_str = String::from_utf8_lossy(&stderr_output); |
| 331 | |||
| 332 | // Check if this is a git protocol error (exit code 128 with stderr) | ||
| 333 | // Protocol errors should be returned as HTTP 200 with ERR pkt-line | ||
| 334 | if is_git_protocol_error(status.code(), &stderr_output) { | ||
| 335 | warn!( | ||
| 336 | "Git receive-pack protocol error (returning ERR pkt-line): {}", | ||
| 337 | stderr_str | ||
| 338 | ); | ||
| 339 | return Ok(build_git_protocol_error_response( | ||
| 340 | GitService::ReceivePack, | ||
| 341 | &stderr_str, | ||
| 342 | )); | ||
| 343 | } | ||
| 344 | |||
| 345 | // Transport errors (spawn failures, signals, etc.) remain as HTTP 500 | ||
| 280 | error!("Git receive-pack failed: {}", stderr_str); | 346 | error!("Git receive-pack failed: {}", stderr_str); |
| 281 | return Err(GitError::GitFailed(status.code())); | 347 | return Err(GitError::GitFailed(status.code())); |
| 282 | } | 348 | } |