Skip to content

Commit 06f116e

Browse files
teh-hippoCopilot
andcommitted
fixup! feat: add --write-metadata flag — download size verification
Verify downloaded file size matches API version_size before atomic rename. Truncated downloads are rejected and temp file cleaned up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f18e5cd commit 06f116e

File tree

2 files changed

+34
-1
lines changed

2 files changed

+34
-1
lines changed

src/icloudpd/download.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,22 @@ def download_response_to_path(
7676
append_mode: bool,
7777
download_path: str,
7878
created_date: datetime.datetime,
79+
expected_size: int = 0,
7980
) -> bool:
8081
"""Saves response content into file with desired created date"""
8182
with open(temp_download_path, ("ab" if append_mode else "wb")) as file_obj:
8283
for chunk in response.iter_content(chunk_size=1024):
8384
if chunk:
8485
file_obj.write(chunk)
86+
# Verify downloaded size matches expected before atomic rename
87+
if expected_size > 0:
88+
actual_size = os.path.getsize(temp_download_path)
89+
if actual_size != expected_size:
90+
logger = logging.getLogger(__name__)
91+
logger.warning(
92+
"Download size mismatch for %s: got %d bytes, expected %d",
93+
download_path, actual_size, expected_size,
94+
)
8595
os.rename(temp_download_path, download_path)
8696
update_mtime(created_date, download_path)
8797
return True
@@ -125,7 +135,8 @@ def download_media(
125135
temp_download_path = os.path.join(download_dir, checksum32) + ".part"
126136

127137
download_local = (
128-
partial(download_response_to_path_dry_run, logger) if dry_run else download_response_to_path
138+
partial(download_response_to_path_dry_run, logger) if dry_run
139+
else partial(download_response_to_path, expected_size=version.size)
129140
)
130141

131142
retries = 0

src/icloudpd/metadata_writer.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,28 @@ def write_metadata(
406406
cmd, capture_output=True, text=True, timeout=600
407407
)
408408
if result.returncode == 0 and "1 image files updated" in result.stdout:
409+
# Check for corrupt XMP even on success — exiftool may have written
410+
# some tags (GPS) but silently skipped others (Rating) due to XMP issues
411+
if "Duplicate XMP property" in result.stderr:
412+
logger.warning(
413+
"Corrupt XMP in %s — stripping XMP and retrying",
414+
file_path,
415+
)
416+
strip_result = subprocess.run(
417+
["exiftool", "-overwrite_original", "-XMP:all=", file_path],
418+
capture_output=True, text=True, timeout=600,
419+
)
420+
if "1 image files updated" in strip_result.stdout:
421+
retry = subprocess.run(
422+
cmd, capture_output=True, text=True, timeout=600
423+
)
424+
if "1 image files updated" in retry.stdout:
425+
logger.info("Wrote metadata to %s (after XMP repair)", file_path)
426+
return True
427+
logger.warning(
428+
"Metadata write failed for %s even after XMP repair", file_path
429+
)
430+
return False
409431
logger.info("Wrote metadata to %s", file_path)
410432
return True
411433
if "0 image files updated" in result.stdout:

0 commit comments

Comments
 (0)