Skip to content
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

Jump to bottom

Fix Bug #3115: Add further driveId length checks when generating a /delta response for a Shared Folder #3116

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

Fix Bug #3115: Add further driveId length checks when generating a /delta response for a Shared Folder #3116

wants to merge 27 commits into from
+464 −164

Conversation

abraunegg
Copy link
Owner

  • Add further personal account driveId length checks when generating a /delta response for a Shared Folder

@abraunegg
Add further driveId length checks
d865d9f 
* Add further personal account driveId length checks when generating a /delta response for a Shared Folder
@abraunegg abraunegg added this to the v2.5.5 milestone Feb 12, 2025
@abraunegg abraunegg linked an issue Feb 12, 2025 that may be closed by this pull request
Bug: Application crash when calculating path due to 15 character driveId API bug #3115
Open
@abraunegg abraunegg changed the title Add further driveId length checks when generating a /delta response for a Shared Folder Fix Issue #3115: Add further driveId length checks when generating a /delta response for a Shared Folder Feb 13, 2025
@abraunegg abraunegg changed the title Fix Issue #3115: Add further driveId length checks when generating a /delta response for a Shared Folder Fix Bug #3115: Add further driveId length checks when generating a /delta response for a Shared Folder Feb 13, 2025
@abraunegg
Merge branch 'master' into fix-issue-3115
c41936d
@abraunegg
Update PR
63bc81e 
* Fix selectRemoteTypeByRemoteDriveId() function to also use the remote item id so that the correct DB record is responded with. This ensures that 'localPathExtension =' reflects the correct local path extension and not some other incorrect local path
* If the 'shared' JSON structure is missing, add a debug message
@abraunegg
Update PR
f6b323c 
* Update PR
@abraunegg
Update PR
f6adfaf 
* Revert selectRemoteTypeByRemoteDriveId change as this impacts Business Shared Folders
@murraybo
Copy link

I do have the issue you try to solve here, too.

The current version of the pull request does not fix it. It still brakes during sync.

One thing that i identified, was that the item id, which contains the drive id, need to be adjusted, too. I hacked this by adding a function:

string testProvidedItemIdForLengthIssue(string objectParentDriveId) {
     auto o = objectParentDriveId.split("!");
     
     string newEntry;
     
     newEntry = to!string(o[0].padLeft('0', 16));
     newEntry = newEntry ~ "!" ~ o[1];

     return newEntry;
 }

and calling it at sync.d:9186

image

This allowed me to continue in syncing the shared drive. Nevertheless it broke later due to missing entries in itemdb.

DEBUG: ONEDRIVE PERSONAL API BUG: The provided online ['parentReference']['driveId'] value is not 16 Characters in length - padding with leading zero's
DEBUG:  - old 'driveId' value = 65fb7c645f8339c
DEBUG:  - new 'driveId' value = 065fb7c645f8339c
DEBUG: Attempting to calculate initial local filesystem path for 065fb7c645f8339c and 065FB7C645F8339C!448
DEBUG: The following generated a broken database tree query:
DEBUG: Drive ID: 065fb7c645f8339c
DEBUG: Item ID: 065FB7C645F8339C!448
DEBUG: ERROR: A database consistency issue has been caught. A --resync is needed to rebuild the database.

At that point the relevant db entries looked like this

sqlite> select * from item where driveId = '65fb7c645f8339c';
65fb7c645f8339c|65FB7C645F8339C!448|...||root|...|...|2021-01-24T11:28:53.296Z|||||||||Y|
65fb7c645f8339c|065FB7C645F8339C!448|...||dir|...|...|2021-01-24T11:28:53.296Z|||||||||Y|
sqlite>

@abraunegg This is, as far as i got on this afternoon, I hope this helps you to fix the issue.

BTW: Thanks for this create tool and the work you put into it.

@abraunegg
Copy link
Owner Author

@murraybo
Unfortunately The 'item id' being 15 characters, whilst technically is not correct (it should also be 16 characters) is not the root cause of the issue.

As you can reproduce this issue, please can you urgently send me via email a complete, un-redacted verbose debug log using this PR in its current state.

Please read https://github.com/abraunegg/onedrive/wiki/Generate-https-debug-log-for-support#standalone-mode to generate this log

@abraunegg abraunegg mentioned this pull request Feb 25, 2025
Open
@abraunegg abraunegg linked an issue Feb 25, 2025 that may be closed by this pull request
Bug: Can't sync OneDrive Personal Shared Folders due to inconsistent API response values #3136
Open
@abraunegg
Update sync.d
47a8685 
* Remove 'sea8cc6beffdb43d7976fbc7da445c639' check
@abraunegg
Update sync.d
c803fb9 
* Add further check before committing to database
@abraunegg abraunegg mentioned this pull request Feb 26, 2025
Open
abraunegg added 2 commits March 2, 2025 13:19
@abraunegg
Update PR
5d836ad 
* Update PR based on testing
@abraunegg
Update sync.d
43ba76c 
* Update PR based on further Shared Folder lowercase in JSON response needing remediation
This was linked to issues Mar 2, 2025
Bug: client crashes after downloading deltas and database optimization #3139
Closed
Bug: AssertError@src/itemdb.d(892): Assertion failure #3143
Open
@abraunegg
Update PR based on further debug logs
acf95ea 
* Update PR based on further debug logs dealing with 15 character 'driveId' for Shared Folder
abraunegg added 3 commits March 7, 2025 10:42
@abraunegg
Update onedrive.d
4fc9234 
* Clean up code comments
@abraunegg
Update itemdb.d
d45ca81 
* As part of this PR, increase DB version to ensure all old records in the DB are purged
@abraunegg
Update sync.d
fae1637 
* Update PR based on sanitising JSON output for debug logging when enabled.
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 7, 2025
View reviewed changes
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@abraunegg
Update util.d
4d4642b 
* Update functions in util.d
@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
abraunegg added 4 commits March 8, 2025 10:57
@abraunegg
Update sync.d
95e30bd 
* Fix typo
@abraunegg
Update util.d
53656b3 
* Simplify isValidUTF8Timestamp()
@abraunegg
Update main.d
5ecef44 
* Debug logging is not correct - missing all single --verbose entries which should be also included when performing a debug capture.
@abraunegg
Update sync.d
3c049a6 
* Based on application debug logs, if /delta fails to send Shared Folders, and the client goes to create these online, we know that these then exist online and are a Shared Folder. Handle in a similar manner to OneDrive Business Shared Folders logic to create the required database tie records if the API fails to send that data originally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.5.5
2 participants
@abraunegg @murraybo