-
Notifications
You must be signed in to change notification settings - Fork 670
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.
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
[Discovery] Fix possible issue with retrieval of permissions from the server #4548
Comments
I have verified the code that reads |
Relevant lines from the logs at https://gist.github.com/nickvergessen/8f5b8ea746a32a7a934b:
|
@nickvergessen This is still confusing. It looks like step03 writes NULL, but step05 reads "RDNVW". Could you verify that this is what's going on by running the test only after step03 and then checking the database? Use something like sqlitebrowser on .csync_journal.db in the sync directory and then look for the "dir" row in the metadata table. What's the value of the remotePerm column? |
The permissions are saved as |
@nickvergessen Thanks! |
I think the problem is this code in
i.e. we don't call I'll try to reproduce the problem and will then fix it. |
In SQLite bindings are not cleared by sqlite3_reset() calls, so skipping a sqlite3_bind call to create a NULL value doesn't work, instead the previous value will be written. To fix this, I clear all bindings in SqlQuery::reset and make sure to explicitly bind NULL when desired in SqlQuery::bind. To make sure there's no confusion about SqlQuery::reset and sqlite3_reset, I rename our method to reset_and_clear_bindings().
I just built a new master client, so it includes 7bd4f95 Fixed the issue and my locking test finally passes in all cases 👍 |
In SQLite bindings are not cleared by sqlite3_reset() calls, so skipping a sqlite3_bind call to create a NULL value doesn't work, instead the previous value will be written. To fix this, I clear all bindings in SqlQuery::reset and make sure to explicitly bind NULL when desired in SqlQuery::bind. To make sure there's no confusion about SqlQuery::reset and sqlite3_reset, I rename our method to reset_and_clear_bindings(). (cherry picked from commit 7bd4f95)
great finding @ckamm and @nickvergessen thanks! |
Detected by @nickvergessen in owncloud/smashbox#134 (comment)
The text was updated successfully, but these errors were encountered: