Skip to content

Hash on stream instead of the entire file #481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
liyimeng wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Hash on stream instead of the entire file #481

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions src/pull/download_model.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ std::string calculate_file_sha256(const std::string& file_path) {
if (!file.is_open()) {
return "";
}

std::vector<unsigned char> hash(picosha2::k_digest_size);
picosha2::hash256(file, hash.begin(), hash.end());
return picosha2::bytes_to_hex_string(hash.begin(), hash.end());
Expand All @@ -34,16 +33,27 @@ std::string calculate_git_blob_oid(const std::string& file_path) {
return "";
}
file.seekg(0, std::ios::end);
size_t size = file.tellg();
file.seekg(0, std::ios::beg);
if (!file) {
return "";
}

std::ostringstream oss;
oss << "blob " << size << '\0'; // Git blob header
oss << file.rdbuf();
std::streampos end_pos = file.tellg();
if (end_pos == std::streampos(-1)) {
return "";
}

std::string blob_data = oss.str();
size_t size = static_cast<size_t>(end_pos);
file.clear();
Comment on lines +40 to +46

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

end_pos (a std::streampos) is cast directly to size_t. On some platforms/configs this can truncate/overflow for large files (e.g., 32-bit builds or very large model files), producing an incorrect Git blob OID and causing spurious hash mismatches. Consider computing the byte size via std::filesystem::file_size() (with error_code) or converting through std::streamoff and explicitly checking end_pos < 0 / end_pos > std::numeric_limits<size_t>::max() before casting.

Copilot uses AI. Check for mistakes.
file.seekg(0, std::ios::beg);
if (!file) {
return "";
}

std::string header = "blob " + std::to_string(size) + '\0';

Copilot AI Apr 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Building the Git blob header as "blob " + std::to_string(size) + '\0' relies on an embedded NUL in a std::string, which is easy to miss during maintenance. Consider constructing the header without the terminator and then push_back('\0') (or equivalent) so it’s more obvious that the NUL is intentional.

Suggested change
std::string header = "blob " + std::to_string(size) + '\0';
std::string header = "blob " + std::to_string(size);
header.push_back('\0');

Copilot uses AI. Check for mistakes.
SHA1 sha1;
sha1.update(blob_data);
sha1.update(header);
// Update SHA1 directly from the file stream (reads from current position to EOF)
sha1.update(file);
return sha1.final();
}

Expand Down Expand Up @@ -186,6 +196,7 @@ bool download_file(const std::string& url, const std::string& local_path, bool i

header_print("FLM", "Checking Hash...");
std::string local_oid = is_lfs ? calculate_file_sha256(local_path) : calculate_git_blob_oid(local_path);

if (local_oid != remote_oid) {
header_print("FLM", "Hash not matched!");
show_cursor(); // Show cursor on error
Expand Down
Loading