mirror of
https://github.com/abraunegg/onedrive.git
synced 2026-10-23 03:08:55 +03:00
Fix modified-file reconciliation when a local file is replaced with different content while preserving an older modification timestamp.
This can occur through normal workflows such as:
* `cp -p`
* `rsync -t`
* restoring files from backup
* file managers that preserve source timestamps
Previously, if the replacement content differed but its local modification timestamp was older than the online timestamp, the client could incorrectly conclude that the online file was newer.
This caused the local replacement to be treated as a conflict even when the online object had not changed since the client's last synchronised state.
The resulting behaviour was:
```text
local content replaced with older preserved mtime
|
v
classified as "online is newer"
|
v
local replacement moved to safeBackup
|
v
safeBackup uploaded as a new file
|
v
existing online version downloaded back
```
This PR corrects that decision while retaining the existing data-preservation behaviour for genuine online changes.
## Root Cause
`performModifiedFileUpload()` already retrieves fresh metadata for the online DriveItem before deciding how to process a modified local file.
The existing conflict logic, however, primarily used:
```text
localModifiedTime < onlineModifiedTime
```
to determine that the online version should take precedence.
That assumption is not sufficient.
A local file can contain new content while legitimately retaining an older timestamp.
Importantly, the client also has another piece of information available at this point: the current online eTag.
Where the freshly retrieved online eTag is unchanged from the eTag stored in the local database, the online object has not changed relative to the client's synchronised baseline.
An older local timestamp alone therefore must not cause the local replacement to be classified as an online conflict.
## Change
The modified-file upload decision now determines whether the current online DriveItem can be proven unchanged from the database baseline.
The database baseline is trusted only when:
* the current online object corresponds to the same DriveItem identity represented by the database entry;
* the online item ID matches the expected target item;
* both the current online eTag and database eTag are present and non-empty;
* the current online eTag matches the database eTag; and
* the client is not operating under `--resync`.
When these conditions are satisfied:
```text
different local content
+ older local timestamp
+ unchanged online eTag
= local modification
```
The existing modified-file upload path is allowed to proceed.
Where the online eTag has changed, is unavailable, cannot be safely associated with the same DriveItem identity, or the client is running under `--resync`, the existing conservative conflict handling remains unchanged.
This intentionally keeps the fix narrowly focused on cases where the client can positively establish that the online object has not changed.
## `remoteItem` Handling
Shared/remote items require additional care because the top-level database item and the actual target DriveItem can represent different identities.
This PR does not attempt to redesign that behaviour.
Where the current online object cannot be safely correlated with the database eTag baseline, the existing data-preservation path remains in effect.
## E2E Test Coverage
The timestamp and replacement E2E coverage has been expanded to exercise actual Microsoft Office XLSX files rather than relying solely on synthetic text payloads.
This is intentional because XLSX files can exercise additional Microsoft Graph and SharePoint behaviour, including SharePoint enrichment of uploaded Office documents.
The existing XLSX implementation previously contained within TC0021 has been centralised into the E2E framework and TC0021 now consumes that common implementation.
The shared XLSX helper supports:
* creation of valid OOXML XLSX files;
* validation of generated/downloaded workbooks;
* mutation of existing workbooks while preserving other ZIP package members; and
* generation of files on both sides of the simple/session upload boundary.
This allows subsequent mutations to operate on the actual workbook returned by Microsoft, including any service-side package changes.
### TC0021
Existing resumable-transfer XLSX testing now uses the common XLSX framework implementation.
The existing large-file/session-upload behaviour remains unchanged.
### TC0029
`--local-first --upload-only` timestamp-preservation testing now uses a real XLSX document.
### TC0036
Overwrite/replace coverage has been expanded substantially.
#### OR-0001 — newer local replacement
A synchronised XLSX is replaced with different XLSX content having a newer local timestamp.
Expected result:
* normal modified-file upload;
* replacement becomes the canonical online content.
#### OR-0002 — older timestamp, simple upload
A synchronised XLSX is replaced using a timestamp-preserving copy operation with:
* different content;
* an older local modification timestamp; and
* no independent online change.
Expected result:
* online eTag still matches the database baseline;
* replacement is processed as a modified file;
* no erroneous safeBackup is created;
* canonical online content becomes the local replacement.
This directly covers the reported failure condition.
#### OR-0003 — older timestamp, session upload
The same scenario as OR-0002 is performed using an XLSX larger than the simple-upload threshold.
Expected result:
* session upload is used;
* modified upload succeeds;
* no erroneous safeBackup is created.
This confirms that the reconciliation fix operates correctly before upload-method selection.
#### OR-0004 — genuine online conflict
A separate mutator client is created using the established E2E multi-client model:
```text
clone tracked configuration/database/local state
|
v
mutator runs --monitor --upload-only
|
v
mutator changes XLSX online
|
v
remote modification independently verified
|
v
stale client receives different local replacement
with older preserved timestamp
```
Expected result:
* current online eTag differs from the stale client's database baseline;
* existing conflict handling remains active;
* the local replacement is preserved through safeBackup;
* the independently changed online version remains authoritative.
This is the safety control demonstrating that the fix does not weaken protection against genuine online changes.
### TC0037
mtime-only local-change handling now uses XLSX files across:
* simple upload-sized files;
* automatic session-upload-sized files;
* forced-session small files; and
* forced-session large files.
The downloaded Microsoft-returned workbook is used as the settled content baseline before timestamp-only mutations are performed.
### TC0051
Monitor-mode mtime-only testing now uses XLSX content and confirms that a timestamp-only local filesystem event does not result in an unnecessary upload or remote content change.
## Scope
This PR does not change the general timestamp authority policy introduced by earlier timestamp work.
It does not change:
* Microsoft timestamp application after successful uploads;
* simple versus session upload selection;
* `--local-first` conflict policy;
* `--resync` semantics;
* genuine online-change conflict preservation; or
* shared/remote item identity handling beyond refusing to trust an ambiguous eTag baseline.
The change is deliberately limited to preventing an older preserved local timestamp from being treated as proof of an online conflict when the current online DriveItem can be positively demonstrated to be unchanged from the client's database baseline.
160 lines
5.9 KiB
Python
160 lines
5.9 KiB
Python
from __future__ import annotations
|
|
|
|
import hashlib
|
|
import os
|
|
import zipfile
|
|
from pathlib import Path
|
|
from xml.sax.saxutils import escape
|
|
|
|
|
|
DOCUMENT_MEMBER = "word/document.xml"
|
|
REQUIRED_MEMBERS = {
|
|
"[Content_Types].xml",
|
|
"_rels/.rels",
|
|
DOCUMENT_MEMBER,
|
|
}
|
|
|
|
|
|
def validate_docx(path: Path, *, required_marker: str | None = None, forbidden_marker: str | None = None) -> str:
|
|
if not path.is_file():
|
|
return f"DOCX file does not exist: {path}"
|
|
|
|
try:
|
|
with zipfile.ZipFile(path, "r") as archive:
|
|
names = set(archive.namelist())
|
|
missing = sorted(REQUIRED_MEMBERS - names)
|
|
if missing:
|
|
return f"DOCX package is missing required members: {', '.join(missing)}"
|
|
corrupt_member = archive.testzip()
|
|
if corrupt_member is not None:
|
|
return f"DOCX package contains corrupt ZIP member: {corrupt_member}"
|
|
document_xml = archive.read(DOCUMENT_MEMBER)
|
|
except (OSError, zipfile.BadZipFile) as exc:
|
|
return f"Unable to validate DOCX package: {exc}"
|
|
|
|
if required_marker is not None and required_marker.encode("utf-8") not in document_xml:
|
|
return f"DOCX document.xml does not contain required marker: {required_marker}"
|
|
if forbidden_marker is not None and forbidden_marker.encode("utf-8") in document_xml:
|
|
return f"DOCX document.xml still contains forbidden marker: {forbidden_marker}"
|
|
|
|
return ""
|
|
|
|
|
|
def docx_contains_marker(path: Path, marker: str) -> bool:
|
|
if not path.is_file():
|
|
return False
|
|
try:
|
|
with zipfile.ZipFile(path, "r") as archive:
|
|
return marker.encode("utf-8") in archive.read(DOCUMENT_MEMBER)
|
|
except (OSError, KeyError, zipfile.BadZipFile):
|
|
return False
|
|
|
|
|
|
def docx_member_sha256(path: Path, member: str = DOCUMENT_MEMBER) -> str:
|
|
with zipfile.ZipFile(path, "r") as archive:
|
|
return hashlib.sha256(archive.read(member)).hexdigest()
|
|
|
|
|
|
def _rewrite_docx_member(path: Path, member: str, new_data: bytes) -> None:
|
|
temp_path = path.with_name(path.name + ".rewriting")
|
|
try:
|
|
with zipfile.ZipFile(path, "r") as source, zipfile.ZipFile(temp_path, "w") as target:
|
|
found = False
|
|
for info in source.infolist():
|
|
data = source.read(info.filename)
|
|
if info.filename == member:
|
|
data = new_data
|
|
found = True
|
|
target.writestr(info, data)
|
|
if not found:
|
|
raise RuntimeError(f"DOCX package does not contain required member: {member}")
|
|
os.replace(temp_path, path)
|
|
finally:
|
|
temp_path.unlink(missing_ok=True)
|
|
|
|
|
|
def append_docx_test_marker(path: Path, marker: str) -> None:
|
|
"""
|
|
Append one deterministic visible paragraph to word/document.xml while retaining
|
|
every other package member exactly as supplied by the Microsoft-settled DOCX.
|
|
|
|
The marker is deliberately reversible without an external recovery file. If a
|
|
CI runner is interrupted after the modified document reaches SharePoint, a later
|
|
run can identify the marker and remove only the paragraph introduced here.
|
|
"""
|
|
validation_error = validate_docx(path)
|
|
if validation_error:
|
|
raise RuntimeError(validation_error)
|
|
|
|
marker_bytes = marker.encode("utf-8")
|
|
with zipfile.ZipFile(path, "r") as archive:
|
|
document_xml = archive.read(DOCUMENT_MEMBER)
|
|
|
|
if marker_bytes in document_xml:
|
|
raise RuntimeError(f"DOCX test marker already exists: {marker}")
|
|
|
|
body_close = b"</w:body>"
|
|
insert_at = document_xml.rfind(body_close)
|
|
if insert_at < 0:
|
|
raise RuntimeError("DOCX document.xml does not contain </w:body>")
|
|
|
|
escaped_marker = escape(marker)
|
|
paragraph = (
|
|
f'<w:p><w:r><w:t xml:space="preserve">{escaped_marker}</w:t></w:r></w:p>'
|
|
).encode("utf-8")
|
|
mutated_xml = document_xml[:insert_at] + paragraph + document_xml[insert_at:]
|
|
_rewrite_docx_member(path, DOCUMENT_MEMBER, mutated_xml)
|
|
|
|
validation_error = validate_docx(path, required_marker=marker)
|
|
if validation_error:
|
|
raise RuntimeError(validation_error)
|
|
|
|
|
|
def remove_docx_test_marker(path: Path, marker: str) -> bool:
|
|
"""
|
|
Remove the paragraph containing the deterministic E2E marker.
|
|
|
|
This first uses the exact paragraph form emitted by append_docx_test_marker().
|
|
A conservative paragraph-boundary fallback is then used in case SharePoint has
|
|
retained the marker but normalised surrounding XML formatting.
|
|
"""
|
|
validation_error = validate_docx(path)
|
|
if validation_error:
|
|
raise RuntimeError(validation_error)
|
|
|
|
marker_bytes = marker.encode("utf-8")
|
|
with zipfile.ZipFile(path, "r") as archive:
|
|
document_xml = archive.read(DOCUMENT_MEMBER)
|
|
|
|
if marker_bytes not in document_xml:
|
|
return False
|
|
if document_xml.count(marker_bytes) != 1:
|
|
raise RuntimeError(
|
|
f"Expected exactly one DOCX test marker during rollback; found {document_xml.count(marker_bytes)}"
|
|
)
|
|
|
|
escaped_marker = escape(marker)
|
|
exact_paragraph = (
|
|
f'<w:p><w:r><w:t xml:space="preserve">{escaped_marker}</w:t></w:r></w:p>'
|
|
).encode("utf-8")
|
|
|
|
if exact_paragraph in document_xml:
|
|
restored_xml = document_xml.replace(exact_paragraph, b"", 1)
|
|
else:
|
|
marker_pos = document_xml.find(marker_bytes)
|
|
paragraph_start = document_xml.rfind(b"<w:p", 0, marker_pos)
|
|
paragraph_end = document_xml.find(b"</w:p>", marker_pos)
|
|
if paragraph_start < 0 or paragraph_end < 0:
|
|
raise RuntimeError(
|
|
"Unable to locate the DOCX paragraph containing the stranded E2E marker"
|
|
)
|
|
paragraph_end += len(b"</w:p>")
|
|
restored_xml = document_xml[:paragraph_start] + document_xml[paragraph_end:]
|
|
|
|
_rewrite_docx_member(path, DOCUMENT_MEMBER, restored_xml)
|
|
|
|
validation_error = validate_docx(path, forbidden_marker=marker)
|
|
if validation_error:
|
|
raise RuntimeError(validation_error)
|
|
return True
|