fix(mercury): parse commit from bytes lost gpgsig#1430
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Collaborator
Author
|
回退代码到之前的稳定版本 |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a parsing issue where the GPG signature field was being lost when parsing commit objects from bytes. The previous implementation incorrectly handled the commit format by skipping GPG signatures and other fields between headers.
Key changes:
- Replaced line-by-line header parsing with sequential field extraction
- Modified parsing logic to preserve GPG signatures and other fields in the commit message
- Added test case to verify GPG signature preservation
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+195
to
197
| let message = unsafe { | ||
| String::from_utf8_unchecked(commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec()) | ||
| }; |
There was a problem hiding this comment.
Using unsafe and from_utf8_unchecked is risky here. Git commit data could contain invalid UTF-8 sequences. Consider using String::from_utf8_lossy or proper UTF-8 validation instead.
Suggested change
| let message = unsafe { | |
| String::from_utf8_unchecked(commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec()) | |
| }; | |
| let message = String::from_utf8_lossy( | |
| &commit[commit.find_byte(0x0a).unwrap() + 1..] | |
| ).to_string(); |
Comment on lines
+152
to
197
| let tree_end = commit.find_byte(0x0a).unwrap(); | ||
| let tree_id: SHA1 = SHA1::from_str( | ||
| String::from_utf8(commit[5..tree_end].to_owned()) // 5 is the length of "tree " | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap(); | ||
| let binding = commit[tree_end + 1..].to_vec(); // Move past the tree id | ||
| commit = &binding; | ||
|
|
||
| // Find the blank line that separates headers from the message | ||
| if let Some(pos) = headers.find(b"\n\n") { | ||
| message_start = pos + 2; | ||
| headers = &headers[..pos]; | ||
| } else { | ||
| // If no blank line, the whole data is headers, no message | ||
| } | ||
| // Find the parent commit ids and remove them from the data | ||
| let author_begin = commit.find("author").unwrap(); | ||
| // Find all parent commit ids | ||
| // The parent commit ids are all the lines that start with "parent " | ||
| // We can use find_iter to find all occurrences of "parent " | ||
| // and then extract the SHA1 hashes from them. | ||
| let parent_commit_ids: Vec<SHA1> = commit[..author_begin] | ||
| .find_iter("parent") | ||
| .map(|parent| { | ||
| let parent_end = commit[parent..].find_byte(0x0a).unwrap(); | ||
| SHA1::from_str( | ||
| // 7 is the length of "parent " | ||
| String::from_utf8(commit[parent + 7..parent + parent_end].to_owned()) | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap() | ||
| }) | ||
| .collect(); | ||
| let binding = commit[author_begin..].to_vec(); | ||
| commit = &binding; | ||
|
|
||
| let mut tree_id: Option<SHA1> = None; | ||
| let mut parent_commit_ids: Vec<SHA1> = Vec::new(); | ||
| let mut author: Option<Signature> = None; | ||
| let mut committer: Option<Signature> = None; | ||
| // Find the author and committer and remove them from the data | ||
| // 0x0a is the newline character | ||
| let author = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| for line in headers.lines() { | ||
| if let Some(tree_str) = line.strip_prefix(b"tree ") { | ||
| tree_id = Some( | ||
| SHA1::from_str(tree_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in tree SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid tree SHA: {}", e)))?, | ||
| ); | ||
| } else if let Some(parent_str) = line.strip_prefix(b"parent ") { | ||
| parent_commit_ids.push( | ||
| SHA1::from_str(parent_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in parent SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid parent SHA: {}", e)))?, | ||
| ); | ||
| } else if line.starts_with(b"author ") { | ||
| author = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid author signature: {}", e)) | ||
| })?); | ||
| } else if line.starts_with(b"committer ") { | ||
| committer = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid committer signature: {}", e)) | ||
| })?); | ||
| } | ||
| } | ||
| let binding = commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec(); | ||
| commit = &binding; | ||
| let committer = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| let message = if message_start > 0 { | ||
| String::from_utf8_lossy(&data[message_start..]).to_string() | ||
| } else { | ||
| String::new() | ||
| // The rest is the message | ||
| let message = unsafe { | ||
| String::from_utf8_unchecked(commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec()) | ||
| }; |
There was a problem hiding this comment.
Multiple calls to unwrap() throughout the parsing logic could cause panics on malformed commit data. The original code had proper error handling. Consider returning appropriate GitError::InvalidCommit variants instead of panicking.
Comment on lines
+152
to
197
| let tree_end = commit.find_byte(0x0a).unwrap(); | ||
| let tree_id: SHA1 = SHA1::from_str( | ||
| String::from_utf8(commit[5..tree_end].to_owned()) // 5 is the length of "tree " | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap(); | ||
| let binding = commit[tree_end + 1..].to_vec(); // Move past the tree id | ||
| commit = &binding; | ||
|
|
||
| // Find the blank line that separates headers from the message | ||
| if let Some(pos) = headers.find(b"\n\n") { | ||
| message_start = pos + 2; | ||
| headers = &headers[..pos]; | ||
| } else { | ||
| // If no blank line, the whole data is headers, no message | ||
| } | ||
| // Find the parent commit ids and remove them from the data | ||
| let author_begin = commit.find("author").unwrap(); | ||
| // Find all parent commit ids | ||
| // The parent commit ids are all the lines that start with "parent " | ||
| // We can use find_iter to find all occurrences of "parent " | ||
| // and then extract the SHA1 hashes from them. | ||
| let parent_commit_ids: Vec<SHA1> = commit[..author_begin] | ||
| .find_iter("parent") | ||
| .map(|parent| { | ||
| let parent_end = commit[parent..].find_byte(0x0a).unwrap(); | ||
| SHA1::from_str( | ||
| // 7 is the length of "parent " | ||
| String::from_utf8(commit[parent + 7..parent + parent_end].to_owned()) | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap() | ||
| }) | ||
| .collect(); | ||
| let binding = commit[author_begin..].to_vec(); | ||
| commit = &binding; | ||
|
|
||
| let mut tree_id: Option<SHA1> = None; | ||
| let mut parent_commit_ids: Vec<SHA1> = Vec::new(); | ||
| let mut author: Option<Signature> = None; | ||
| let mut committer: Option<Signature> = None; | ||
| // Find the author and committer and remove them from the data | ||
| // 0x0a is the newline character | ||
| let author = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| for line in headers.lines() { | ||
| if let Some(tree_str) = line.strip_prefix(b"tree ") { | ||
| tree_id = Some( | ||
| SHA1::from_str(tree_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in tree SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid tree SHA: {}", e)))?, | ||
| ); | ||
| } else if let Some(parent_str) = line.strip_prefix(b"parent ") { | ||
| parent_commit_ids.push( | ||
| SHA1::from_str(parent_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in parent SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid parent SHA: {}", e)))?, | ||
| ); | ||
| } else if line.starts_with(b"author ") { | ||
| author = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid author signature: {}", e)) | ||
| })?); | ||
| } else if line.starts_with(b"committer ") { | ||
| committer = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid committer signature: {}", e)) | ||
| })?); | ||
| } | ||
| } | ||
| let binding = commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec(); | ||
| commit = &binding; | ||
| let committer = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| let message = if message_start > 0 { | ||
| String::from_utf8_lossy(&data[message_start..]).to_string() | ||
| } else { | ||
| String::new() | ||
| // The rest is the message | ||
| let message = unsafe { | ||
| String::from_utf8_unchecked(commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec()) | ||
| }; |
There was a problem hiding this comment.
Multiple calls to unwrap() throughout the parsing logic could cause panics on malformed commit data. The original code had proper error handling. Consider returning appropriate GitError::InvalidCommit variants instead of panicking.
Comment on lines
+152
to
197
| let tree_end = commit.find_byte(0x0a).unwrap(); | ||
| let tree_id: SHA1 = SHA1::from_str( | ||
| String::from_utf8(commit[5..tree_end].to_owned()) // 5 is the length of "tree " | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap(); | ||
| let binding = commit[tree_end + 1..].to_vec(); // Move past the tree id | ||
| commit = &binding; | ||
|
|
||
| // Find the blank line that separates headers from the message | ||
| if let Some(pos) = headers.find(b"\n\n") { | ||
| message_start = pos + 2; | ||
| headers = &headers[..pos]; | ||
| } else { | ||
| // If no blank line, the whole data is headers, no message | ||
| } | ||
| // Find the parent commit ids and remove them from the data | ||
| let author_begin = commit.find("author").unwrap(); | ||
| // Find all parent commit ids | ||
| // The parent commit ids are all the lines that start with "parent " | ||
| // We can use find_iter to find all occurrences of "parent " | ||
| // and then extract the SHA1 hashes from them. | ||
| let parent_commit_ids: Vec<SHA1> = commit[..author_begin] | ||
| .find_iter("parent") | ||
| .map(|parent| { | ||
| let parent_end = commit[parent..].find_byte(0x0a).unwrap(); | ||
| SHA1::from_str( | ||
| // 7 is the length of "parent " | ||
| String::from_utf8(commit[parent + 7..parent + parent_end].to_owned()) | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap() | ||
| }) | ||
| .collect(); | ||
| let binding = commit[author_begin..].to_vec(); | ||
| commit = &binding; | ||
|
|
||
| let mut tree_id: Option<SHA1> = None; | ||
| let mut parent_commit_ids: Vec<SHA1> = Vec::new(); | ||
| let mut author: Option<Signature> = None; | ||
| let mut committer: Option<Signature> = None; | ||
| // Find the author and committer and remove them from the data | ||
| // 0x0a is the newline character | ||
| let author = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| for line in headers.lines() { | ||
| if let Some(tree_str) = line.strip_prefix(b"tree ") { | ||
| tree_id = Some( | ||
| SHA1::from_str(tree_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in tree SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid tree SHA: {}", e)))?, | ||
| ); | ||
| } else if let Some(parent_str) = line.strip_prefix(b"parent ") { | ||
| parent_commit_ids.push( | ||
| SHA1::from_str(parent_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in parent SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid parent SHA: {}", e)))?, | ||
| ); | ||
| } else if line.starts_with(b"author ") { | ||
| author = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid author signature: {}", e)) | ||
| })?); | ||
| } else if line.starts_with(b"committer ") { | ||
| committer = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid committer signature: {}", e)) | ||
| })?); | ||
| } | ||
| } | ||
| let binding = commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec(); | ||
| commit = &binding; | ||
| let committer = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| let message = if message_start > 0 { | ||
| String::from_utf8_lossy(&data[message_start..]).to_string() | ||
| } else { | ||
| String::new() | ||
| // The rest is the message | ||
| let message = unsafe { | ||
| String::from_utf8_unchecked(commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec()) | ||
| }; |
There was a problem hiding this comment.
Multiple calls to unwrap() throughout the parsing logic could cause panics on malformed commit data. The original code had proper error handling. Consider returning appropriate GitError::InvalidCommit variants instead of panicking.
Comment on lines
+152
to
197
| let tree_end = commit.find_byte(0x0a).unwrap(); | ||
| let tree_id: SHA1 = SHA1::from_str( | ||
| String::from_utf8(commit[5..tree_end].to_owned()) // 5 is the length of "tree " | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap(); | ||
| let binding = commit[tree_end + 1..].to_vec(); // Move past the tree id | ||
| commit = &binding; | ||
|
|
||
| // Find the blank line that separates headers from the message | ||
| if let Some(pos) = headers.find(b"\n\n") { | ||
| message_start = pos + 2; | ||
| headers = &headers[..pos]; | ||
| } else { | ||
| // If no blank line, the whole data is headers, no message | ||
| } | ||
| // Find the parent commit ids and remove them from the data | ||
| let author_begin = commit.find("author").unwrap(); | ||
| // Find all parent commit ids | ||
| // The parent commit ids are all the lines that start with "parent " | ||
| // We can use find_iter to find all occurrences of "parent " | ||
| // and then extract the SHA1 hashes from them. | ||
| let parent_commit_ids: Vec<SHA1> = commit[..author_begin] | ||
| .find_iter("parent") | ||
| .map(|parent| { | ||
| let parent_end = commit[parent..].find_byte(0x0a).unwrap(); | ||
| SHA1::from_str( | ||
| // 7 is the length of "parent " | ||
| String::from_utf8(commit[parent + 7..parent + parent_end].to_owned()) | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap() | ||
| }) | ||
| .collect(); | ||
| let binding = commit[author_begin..].to_vec(); | ||
| commit = &binding; | ||
|
|
||
| let mut tree_id: Option<SHA1> = None; | ||
| let mut parent_commit_ids: Vec<SHA1> = Vec::new(); | ||
| let mut author: Option<Signature> = None; | ||
| let mut committer: Option<Signature> = None; | ||
| // Find the author and committer and remove them from the data | ||
| // 0x0a is the newline character | ||
| let author = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| for line in headers.lines() { | ||
| if let Some(tree_str) = line.strip_prefix(b"tree ") { | ||
| tree_id = Some( | ||
| SHA1::from_str(tree_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in tree SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid tree SHA: {}", e)))?, | ||
| ); | ||
| } else if let Some(parent_str) = line.strip_prefix(b"parent ") { | ||
| parent_commit_ids.push( | ||
| SHA1::from_str(parent_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in parent SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid parent SHA: {}", e)))?, | ||
| ); | ||
| } else if line.starts_with(b"author ") { | ||
| author = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid author signature: {}", e)) | ||
| })?); | ||
| } else if line.starts_with(b"committer ") { | ||
| committer = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid committer signature: {}", e)) | ||
| })?); | ||
| } | ||
| } | ||
| let binding = commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec(); | ||
| commit = &binding; | ||
| let committer = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| let message = if message_start > 0 { | ||
| String::from_utf8_lossy(&data[message_start..]).to_string() | ||
| } else { | ||
| String::new() | ||
| // The rest is the message | ||
| let message = unsafe { | ||
| String::from_utf8_unchecked(commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec()) | ||
| }; |
There was a problem hiding this comment.
Multiple calls to unwrap() throughout the parsing logic could cause panics on malformed commit data. The original code had proper error handling. Consider returning appropriate GitError::InvalidCommit variants instead of panicking.
Comment on lines
+152
to
197
| let tree_end = commit.find_byte(0x0a).unwrap(); | ||
| let tree_id: SHA1 = SHA1::from_str( | ||
| String::from_utf8(commit[5..tree_end].to_owned()) // 5 is the length of "tree " | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap(); | ||
| let binding = commit[tree_end + 1..].to_vec(); // Move past the tree id | ||
| commit = &binding; | ||
|
|
||
| // Find the blank line that separates headers from the message | ||
| if let Some(pos) = headers.find(b"\n\n") { | ||
| message_start = pos + 2; | ||
| headers = &headers[..pos]; | ||
| } else { | ||
| // If no blank line, the whole data is headers, no message | ||
| } | ||
| // Find the parent commit ids and remove them from the data | ||
| let author_begin = commit.find("author").unwrap(); | ||
| // Find all parent commit ids | ||
| // The parent commit ids are all the lines that start with "parent " | ||
| // We can use find_iter to find all occurrences of "parent " | ||
| // and then extract the SHA1 hashes from them. | ||
| let parent_commit_ids: Vec<SHA1> = commit[..author_begin] | ||
| .find_iter("parent") | ||
| .map(|parent| { | ||
| let parent_end = commit[parent..].find_byte(0x0a).unwrap(); | ||
| SHA1::from_str( | ||
| // 7 is the length of "parent " | ||
| String::from_utf8(commit[parent + 7..parent + parent_end].to_owned()) | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap() | ||
| }) | ||
| .collect(); | ||
| let binding = commit[author_begin..].to_vec(); | ||
| commit = &binding; | ||
|
|
||
| let mut tree_id: Option<SHA1> = None; | ||
| let mut parent_commit_ids: Vec<SHA1> = Vec::new(); | ||
| let mut author: Option<Signature> = None; | ||
| let mut committer: Option<Signature> = None; | ||
| // Find the author and committer and remove them from the data | ||
| // 0x0a is the newline character | ||
| let author = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| for line in headers.lines() { | ||
| if let Some(tree_str) = line.strip_prefix(b"tree ") { | ||
| tree_id = Some( | ||
| SHA1::from_str(tree_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in tree SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid tree SHA: {}", e)))?, | ||
| ); | ||
| } else if let Some(parent_str) = line.strip_prefix(b"parent ") { | ||
| parent_commit_ids.push( | ||
| SHA1::from_str(parent_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in parent SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid parent SHA: {}", e)))?, | ||
| ); | ||
| } else if line.starts_with(b"author ") { | ||
| author = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid author signature: {}", e)) | ||
| })?); | ||
| } else if line.starts_with(b"committer ") { | ||
| committer = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid committer signature: {}", e)) | ||
| })?); | ||
| } | ||
| } | ||
| let binding = commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec(); | ||
| commit = &binding; | ||
| let committer = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| let message = if message_start > 0 { | ||
| String::from_utf8_lossy(&data[message_start..]).to_string() | ||
| } else { | ||
| String::new() | ||
| // The rest is the message | ||
| let message = unsafe { | ||
| String::from_utf8_unchecked(commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec()) | ||
| }; |
There was a problem hiding this comment.
Multiple calls to unwrap() throughout the parsing logic could cause panics on malformed commit data. The original code had proper error handling. Consider returning appropriate GitError::InvalidCommit variants instead of panicking.
Comment on lines
+150
to
+196
| let mut commit = data; | ||
| // Find the tree id and remove it from the data | ||
| let tree_end = commit.find_byte(0x0a).unwrap(); | ||
| let tree_id: SHA1 = SHA1::from_str( | ||
| String::from_utf8(commit[5..tree_end].to_owned()) // 5 is the length of "tree " | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap(); | ||
| let binding = commit[tree_end + 1..].to_vec(); // Move past the tree id | ||
| commit = &binding; | ||
|
|
||
| // Find the blank line that separates headers from the message | ||
| if let Some(pos) = headers.find(b"\n\n") { | ||
| message_start = pos + 2; | ||
| headers = &headers[..pos]; | ||
| } else { | ||
| // If no blank line, the whole data is headers, no message | ||
| } | ||
| // Find the parent commit ids and remove them from the data | ||
| let author_begin = commit.find("author").unwrap(); | ||
| // Find all parent commit ids | ||
| // The parent commit ids are all the lines that start with "parent " | ||
| // We can use find_iter to find all occurrences of "parent " | ||
| // and then extract the SHA1 hashes from them. | ||
| let parent_commit_ids: Vec<SHA1> = commit[..author_begin] | ||
| .find_iter("parent") | ||
| .map(|parent| { | ||
| let parent_end = commit[parent..].find_byte(0x0a).unwrap(); | ||
| SHA1::from_str( | ||
| // 7 is the length of "parent " | ||
| String::from_utf8(commit[parent + 7..parent + parent_end].to_owned()) | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap() | ||
| }) | ||
| .collect(); | ||
| let binding = commit[author_begin..].to_vec(); | ||
| commit = &binding; | ||
|
|
||
| let mut tree_id: Option<SHA1> = None; | ||
| let mut parent_commit_ids: Vec<SHA1> = Vec::new(); | ||
| let mut author: Option<Signature> = None; | ||
| let mut committer: Option<Signature> = None; | ||
| // Find the author and committer and remove them from the data | ||
| // 0x0a is the newline character | ||
| let author = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| for line in headers.lines() { | ||
| if let Some(tree_str) = line.strip_prefix(b"tree ") { | ||
| tree_id = Some( | ||
| SHA1::from_str(tree_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in tree SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid tree SHA: {}", e)))?, | ||
| ); | ||
| } else if let Some(parent_str) = line.strip_prefix(b"parent ") { | ||
| parent_commit_ids.push( | ||
| SHA1::from_str(parent_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in parent SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid parent SHA: {}", e)))?, | ||
| ); | ||
| } else if line.starts_with(b"author ") { | ||
| author = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid author signature: {}", e)) | ||
| })?); | ||
| } else if line.starts_with(b"committer ") { | ||
| committer = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid committer signature: {}", e)) | ||
| })?); | ||
| } | ||
| } | ||
| let binding = commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec(); | ||
| commit = &binding; | ||
| let committer = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| let message = if message_start > 0 { | ||
| String::from_utf8_lossy(&data[message_start..]).to_string() | ||
| } else { | ||
| String::new() | ||
| // The rest is the message | ||
| let message = unsafe { | ||
| String::from_utf8_unchecked(commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec()) |
There was a problem hiding this comment.
Multiple intermediate Vec allocations and reassignments are inefficient. Consider using slice indices to track position in the original data instead of repeatedly copying and reallocating.
Comment on lines
+181
to
+182
| let binding = commit[author_begin..].to_vec(); | ||
| commit = &binding; |
There was a problem hiding this comment.
Multiple intermediate Vec allocations and reassignments are inefficient. Consider using slice indices to track position in the original data instead of repeatedly copying and reallocating.
Comment on lines
+150
to
+196
| let mut commit = data; | ||
| // Find the tree id and remove it from the data | ||
| let tree_end = commit.find_byte(0x0a).unwrap(); | ||
| let tree_id: SHA1 = SHA1::from_str( | ||
| String::from_utf8(commit[5..tree_end].to_owned()) // 5 is the length of "tree " | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap(); | ||
| let binding = commit[tree_end + 1..].to_vec(); // Move past the tree id | ||
| commit = &binding; | ||
|
|
||
| // Find the blank line that separates headers from the message | ||
| if let Some(pos) = headers.find(b"\n\n") { | ||
| message_start = pos + 2; | ||
| headers = &headers[..pos]; | ||
| } else { | ||
| // If no blank line, the whole data is headers, no message | ||
| } | ||
| // Find the parent commit ids and remove them from the data | ||
| let author_begin = commit.find("author").unwrap(); | ||
| // Find all parent commit ids | ||
| // The parent commit ids are all the lines that start with "parent " | ||
| // We can use find_iter to find all occurrences of "parent " | ||
| // and then extract the SHA1 hashes from them. | ||
| let parent_commit_ids: Vec<SHA1> = commit[..author_begin] | ||
| .find_iter("parent") | ||
| .map(|parent| { | ||
| let parent_end = commit[parent..].find_byte(0x0a).unwrap(); | ||
| SHA1::from_str( | ||
| // 7 is the length of "parent " | ||
| String::from_utf8(commit[parent + 7..parent + parent_end].to_owned()) | ||
| .unwrap() | ||
| .as_str(), | ||
| ) | ||
| .unwrap() | ||
| }) | ||
| .collect(); | ||
| let binding = commit[author_begin..].to_vec(); | ||
| commit = &binding; | ||
|
|
||
| let mut tree_id: Option<SHA1> = None; | ||
| let mut parent_commit_ids: Vec<SHA1> = Vec::new(); | ||
| let mut author: Option<Signature> = None; | ||
| let mut committer: Option<Signature> = None; | ||
| // Find the author and committer and remove them from the data | ||
| // 0x0a is the newline character | ||
| let author = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| for line in headers.lines() { | ||
| if let Some(tree_str) = line.strip_prefix(b"tree ") { | ||
| tree_id = Some( | ||
| SHA1::from_str(tree_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in tree SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid tree SHA: {}", e)))?, | ||
| ); | ||
| } else if let Some(parent_str) = line.strip_prefix(b"parent ") { | ||
| parent_commit_ids.push( | ||
| SHA1::from_str(parent_str.to_str().map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid UTF-8 in parent SHA: {}", e)) | ||
| })?) | ||
| .map_err(|e| GitError::InvalidCommit(format!("Invalid parent SHA: {}", e)))?, | ||
| ); | ||
| } else if line.starts_with(b"author ") { | ||
| author = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid author signature: {}", e)) | ||
| })?); | ||
| } else if line.starts_with(b"committer ") { | ||
| committer = Some(Signature::from_data(line.to_vec()).map_err(|e| { | ||
| GitError::InvalidCommit(format!("Invalid committer signature: {}", e)) | ||
| })?); | ||
| } | ||
| } | ||
| let binding = commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec(); | ||
| commit = &binding; | ||
| let committer = | ||
| Signature::from_data(commit[..commit.find_byte(0x0a).unwrap()].to_vec()).unwrap(); | ||
|
|
||
| let message = if message_start > 0 { | ||
| String::from_utf8_lossy(&data[message_start..]).to_string() | ||
| } else { | ||
| String::new() | ||
| // The rest is the message | ||
| let message = unsafe { | ||
| String::from_utf8_unchecked(commit[commit.find_byte(0x0a).unwrap() + 1..].to_vec()) |
There was a problem hiding this comment.
Multiple intermediate Vec allocations and reassignments are inefficient. Consider using slice indices to track position in the original data instead of repeatedly copying and reallocating.
| let parent_end = commit[parent..].find_byte(0x0a).unwrap(); | ||
| SHA1::from_str( | ||
| // 7 is the length of "parent " | ||
| String::from_utf8(commit[parent + 7..parent + parent_end].to_owned()) |
There was a problem hiding this comment.
The slice bounds parent + parent_end is incorrect. parent_end is already an offset from commit[parent..], so this should be parent + 7..parent + 7 + parent_end or restructure the logic to avoid this confusion.
Suggested change
| String::from_utf8(commit[parent + 7..parent + parent_end].to_owned()) | |
| String::from_utf8(commit[parent + 7..parent + 7 + parent_end].to_owned()) |
genedna
enabled auto-merge
September 10, 2025 08:52
genedna
approved these changes
Sep 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.