fopen does not work well with relative path tand forward slashes on windows
This fix the windows textexcludedfiles test.
And also make the code simpler.
Note that the 'trimmed' might be a behavior change, but i think it is ok
If the server has the 'uploadConflictFiles' capability conflict
files will be uploaded instead of ignored.
Uploaded conflict files have the following headers set during upload
OC-Conflict: 1
OC-ConflictBaseFileId: 172489174instanceid
OC-ConflictBaseMtime: 1235789213
OC-ConflictBaseEtag: myetag
when the data is available. Downloads accept the same headers in return
when downloading a conflict file.
In the absence of server support clients will identify conflict files
through the file name pattern and attempt to deduce the base fileid.
Base etag and mtime can't be deduced though.
The upload job for a new conflict file will be triggered directly from
the job that created the conflict file now. No second sync run is
necessary anymore.
This commit does not yet introduce a 'username' like identifier that
automatically gets added to conflict file filenames (to name the files
foo_conflict-Fred-1345.txt instead of just foo_conflict-1345.txt).
Previously, there was csync_ftw_type_e and SyncFileItem::Type. Having
two enums lead to a bug where Type::Unknown == Type::File that went
unnoticed for a good while.
This patch keeps only a single enum.
MSVC stores the enum as signed in the bitfield (contrary to the C++ spec)
Which means that once we store a value such as SyncFileItem::DetailError
in a bitfield, we get a negative value back, then of course, further
comparison fails.
If the application binary is not installed in /usr/bin the client
with this patch considers to check the relative location
../../etc/owncloud-client/ to find the system exclude.
This is an important bit for AppImage based packages of the client,
as this runs from a temporar mountpoint and the system file can not
be found under /etc.
This can happen if the upload of a file is finished, but we just got
disconnected right before recieving the reply containing the etag.
So nothing was save din the DB, and we are not sure if the server
recieved the file properly or not. Further local update of the file
will cause a conflict.
In order to fix this, store the checksum of the uploading file in
the uploadinfo table of the local db (even if there is no chunking
involved). And when we have a conflict, check that it is not because
of this situation by checking the entry in the uploadinfo table.
Issue #5106
This commit integrates support for libcloudproviders
desktop integration API. If build with the library it
will check on startup if the DBus interface is available
and then use it instead of the legacy status icon.
Signed-off-by: Julius Härtl <jus@bitgrid.net>
In addition to using the right function when retrieving inodes this
*also* fixes a more general bug ownsql had with storing uint64 values
that didn't fit into an int64.
The upload is made in an event loop with more than one
upload at the same time, this confuses the hell out of the
folder locking mechanism.
We need to lock the folder and ask the other trials to try
again in a few seconds in the future to give time for the
uploader to actually upload the current file that's locking
the folder.
Also use appName instead of appNameGui in order to compute the path
Issue: #2245
The reason is to respect the XDG spec on Unix (#1601) and might help
on windows roaming profiles (#684)
csync_exclude.cpp:428:17: error: assigning to 'char *' from incompatible type 'const char *'
bname = path;
^~~~
The C library's strrchr always return 'char*'
Only the C++'s std::strrchr has two overloads
Improves full matches by more than an order of magnitude
and also improves speed of traversal matches by roughly 20%,
judging by the check_csync_exclude performance test.
Make ExcludedFiles something that is instantiated outside of
the CSYNC context and then given to it as a hook.
ExcludedFiles still lives in csync_exclude and the internal
workings haven't been touched.
For duplicate file ids the update phase and reconcile phase determined
the rename mappings independently. If they disagreed (due to different
order of processing), complicated misbehavior would result.
This patch fixes it by letting reconcile try to use the mapping that the
update phase has computed first.
Add a new member for the UploadFileInfo in PropagateUploadCommon
to hold the full file path - as it can change if we use a temporary
file to upload.
Adapt propagateuploadv1 to use the new calls.
They can be conceptually equal - I can upload the file
on disk, and that's what I do right now. But if we want
to accept filters in the future, filters that change
the file on disk like shrinking an image, the current
information used is wrong and we need a way to separate those.
This patch introduces a new struct that holds the *actual*
file that will be uploaded, be it a temporary one or
the original file.
* Store privatekey, certificate and mnemonic in keychain
* Retrieve private + public key from server
- ask for mnemonic to decrypt private key
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
* Check for cert + privateKey in keychain
* Work with QSslKey and QSslCertificate
* Abstract reading the BIO's a bit more
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Also, Do not create variables in the heap to change it's value
via reference, prefer an aggregation value. use a Typedef to
fully specify what you want in return.
The problem here is that the QPainter is created on the viewport with is a
QWidget, but QAbstractItemView can have a different font, and the
QStyleOptionViewItem::font is this font. QStyleOptionViewItem::font
was used to compute the sizeHint, and the default font from the QPainter
was used to draw the text, so they could be not in sync.
Fix it by always using the font for QPushButton
Reported in
https://github.com/owncloud/client/issues/6156#issuecomment-346576328
This means we cannot use QtGui in libsync.
So this mostly disable the avatar from the account and the avatarjob
Note that there is one logic change: in ConnectionValidator::slotUserFetched
we do the avatar job even if the user is empty. Otherwise we would end up in
a invalid state. This restore the 2.3.x behavior that was broken in
commit e05d6bfcdc
This is to move generic encryption methods out of the main code and into
small helper functions. So we don't scatter the encryption code all over
the place.
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
This is mainly for linux, whose local is not UTF-8.
For example, in latin1, it is not possible to encode emoji or chinese character.
If there are such character in the filename, Qt would just save the file using
the replacement character ('?'). Then, on the next sync, client would rename
the files using this replacement character.
Avoid this by ignoring the files which cannot be downloaded because the
filename cannot be represented with the user's locale
Relates to issue #5676 and #5719
* Drop AvatarJob2
* Allow AvatarJob to retrieve different sizes and users
* Make creating a circular avatar into a function
(maybe all avatars should be made into that shape in the first place)
[Sharing] Show placeholders for avatars
Just like on the web show placeholders for avatars in the sharing dialog
[Sharing] Show avatars!
[Sharing] Show same avatar placeholder for group/federated shares as on
web
To do this conveniently a bunch of functionality that's common to
IssueWidget and ProtocolWidget is moved to ProtocolItem.
Also the convenience function to asynchronously retrieve the private
link url is moved from the socket api to the network jobs.
The menu can be open with the keyboard shortcut.
(Fixup for the fix of #5596)
Also use popup instead of exec to show the menu: it's safer as it does
not re-enter the event loop.
Previously we required matching mtimes but that's actually
unnecessary when the question is about whether to skip the
download. We will still update the file's metadata.
Also, adjust behavior when the checksum is weak (Adler32):
in these cases we still depend on equal mtimes.
Some servers have non-compliant instance ids (that start with a number)
and thereby make deducing the numeric file id from the full id
unreliable.
To circumvent this problem we retrieve the fileid property from the
server with a PROPFIND.
This restores 2.3 behavior. Some servers reply 404 to GETs and PROPFINDs
to the remote.php/webdav/ url and used to work. Being more picky would
break them.
The assert was made fatal when looking at asserts for #5429#5518
without having a particular problem in mind. Recent reports weakly
suggest that this might lead to occasional crashes for users when
sqlite_close fails in ways that look ignorable.
This is important as a lot of the code would start
to rely in direct access to the client side encryption
and there are different keys for different accounts.
With some firewalls we can't GET /remote.php/webdav/. Here we keep the
GET request to detect shibboleth through the redirect pattern but then
use PROPFIND to figure out the http auth method.
Currently we prefer OAuth to Shibboleth to Basic auth.
This also restores the fallback behavior of assuming basic auth
when no auth type can be determined.
It appears that Qt implementation of the DELETE http request
does not send bodyData, and we need that for Nextcloud.
Currently I changed the http request on the server side
to accept a POST instead of a DELETE, so I can actually
develop.
Also, I already poked the Qt developers that did this code.
Also, commented out the finalization of the decrypt operation
because that was messing with the encryption. There's something
wrong here but I need to get this working and I can fix stuff
later.
For historical reasons CheckServerJob doesn't just check url/ but also
url/owncloud/. However, that means if url/status.php is a 404 and
url/owncloud/status.php is a 404, the user will see the latter url
appear in the error message. That's potentially confusing.
Instead, just show the account url which will be closer to what the
user typed into the account wizard, while being adjusted for protocol
and possible redirects.
* Do not use AAD
* Do not try to decrypt the last 16 bytes as Android adds the tag there
by default
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
* Do not use padding
* Do not use the ADD data
* Append the tag to the ciphertext to be compatible with Android
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
This allow the sync engine to query the new metadata and update the
overlay icons.
Note: we also need to invalidate the etags because the server does not
change the etag of parent directories that see their share-types changed.
Issue #6098
... even if the file is not changed.
We get an UPDATE_METADATA in that case, so make sure we let the
SyncFileStatusTracker know about it.
That means we need to filter out UPDATE_METADATA in the other listeners
of this signal.
Issue #6098
When users share the same tree several times (say A/ and A/B/ are both
shared) the remote tree can have several entries that have the same
file id. This needs to be respected in rename detection.
Also adds several tests and fixes for issues noticed during testing.
See #6096
We mostly trust the file watchers meaning that we don't re-scan the
local tree if we have done that recently and no file watcher events
have arrived. If the file watchers invalidate a subtree, we rescan
only that subtree.
Since we're not entirely sure the file watchers are reliable, we still
do full local discoveries regularly (1h by default). There is a config
file setting as well as an environment variable to control the interval.
Add state and signal to catch the following two known problems:
* Linux: inotify user watch pool is exhausted. Folder watcher becomes
unreliable.
* Windows: buffer is too small, some notifications are lost but watching
stays reliable.
This makes it unnecessary for FolderMan to manage the list and removes
the need for some forwarders.
This is done in preparation for follow-up commits that want to add
diagnostics to FolderWatcher that shall be available from within Folder.
Unfortunately checking the base-url for redirects in all cases lead
to incorrect behavior in some SAML/OAuth2 edge cases.
This new iteration checks the base url for redirects only if the
standard CheckServerJob can't reach the server. That way the 2.3
behavior is only changed in cases that would have lead to errors.
See #5954
Issue #6105
Dynamically find the browser trough topLevelWidgets instead of
forwarding the call to the relevant page as it would require to break
many abstration layers (OwncloudSetupWizard -> OwncloudWizard ->
AbstractCredentialsWizardPage -> OwncloudShibbolethCredsPage)
And considering that we want to phase shibboleth down, I tought is
was not worth adding an interface for this.
The OAuth page don't have this problem because it shows a label and
allow the user to re-open the browser.
For some reason, this was working untill I added a call
to X509_REQ_get_subject_name, then the linking suddenly
stopped working (even tougth I'm using a ton of other
OpenSSL calls)
Force to link against 1.0
This network job does a DELETE http request on a URL. It's the
second class that does basically the same, but this one returns
the http return code, and it's set to do a api call.
If the server supports client syde encryption, display
a menu on right click that should display encrypt and decrypt.
ideally it would show the encrypt if the folder is decrypted, and
decrypt if the folder is encrypted but currently there's no way
for the client to know that.
We did not set a log handler so there were no timestamps.
The --debug didn't have an effect, let's use --logdebug like in GUI version.
(Command line always outputs some log)
Found in owncloud/documentation#3436
The issues tab uses custom ordering where overall and summary sync
issues are displayed first. This ordering is preserved by creating
special sorting logic for the "time" column.
It needed special handling anyway, since sorting by time-string would
have yielded incorrect results.
Previously it could only display synchronization progress or "up to
date". Now it also communicates the same overall state that the icon
shows.
See owncloud/enterprise#2134
Comparison of file sizes for potential conflicts was added in
0eb9401c62, but did not extend to checking
the file size in case of potential local moves.
This commit adds this check and adds tests for various move+change
scenarios.
On Mac, this halves the time spent in csync_excluded_traversal
when using check_csync_excluded_performance. A similar performance
increase is seen on linux.
Fix performence regression from commit d66c2b5fae
For every new file we would look up every parent directories. Allocating
a new QByteArreay for every parent riectory just to know if it is in the other
tree is wasting lots of CPU.
Use a ByteArrayRef trick, similar to QStringRef
To reproduce, log in and click "authorize" on the browser, then close
the browser before the client has replied, (but after redirected to localhost,
i.e. when the client is asking the server for the token)
The problem is that socket can be destroyed so we don't need to answer on a
destroyed socket.
This gets rid of the csync_statedb sqlite layer and use
the same code and same connection as the rest of the SyncEngine.
Missing functions are added to SyncJournalDb and change a few minor
things (like changing SyncJournalFileRecord::_modtime to be an int64
instead of a QDateTime, like it was in csync).
The current implementation would return the same value whether the query failed
or if no row would be found. This is something that is currently checked by csync
and needs to be provided if we want to use SyncJournalDB there.
Adjusted all call sites to also check the return value even though they
could still just rely on rec.isValid(), but makes it more explicit as to what
happens for database errors in those cases, if we ever want to gracefully handle
them.
We need to use concatPath to avoid possible double '/' in the URLs if the
account url() ends with '/'.
This has become even more of a problem since commit
d1b8370a4a which was resolving the url after
a redirect where most server actually add a '/' if the url is a folder
For the remote this was already done long time ago
For windows, this was already partially done
The goal is to avoid needless conversion of the path to local encoding.
Create a specific type that parses the permissions so we can store
it in a short rather than in a QByteArray
Note: in RemotePermissions::toString, we make sure the string is not
empty by adding a space, this was already existing before commit
e8f7adc7ca where it was removed by mistake.
Some slot were protected or private but needed to be public.
Some needed a static_cast (can't use qOverload because it is in Qt 5.7)
This is not only a partial change.
This is motivated by the fact that QMetaObject::noralizeSignature takes 7.35%
CPU of the LargeSyncBench. (Mostly from ABstractNetworkJob::setupConnections and
PropagateUploadFileV1::startNextChunk). It could be fixed by using normalized
signature in the connection statement, but i tought it was a good oportunity
to modernize the code.
This commit only contains calls that were automatically converted with clazy.
Set OWNCLOUD_UPLOAD_CONFLICT_FILES=1 to trigger this behavior.
Note that this is experimental and unsupported. The real feature is
likely to end up in 2.5.
Uploading conflict files is simply done by removing the pattern from
csync_exclude. The rest here deals with making the conflict notification
ui approximately work.
There are still some concerns about where an uploaded conflict file
appears in the sync protocol and issues list (it should be in both, but
is only in one of them currently!).
See #4557.
* The sharing ui does a propfind anyway: use that to query the new
property as well!
* For the socket api, asynchronously query the server for the right url
when an action that needs it is triggered.
The old, manually generated URL will be used as fallback in case the
server doesn't support the new property or the property can't be
retrieved for some reason.
Depends on owncloud/core#29021
... or child folders
There is also no real reason to forbid the user from syncing the same
folder to multiple location on its hardrive.
A real use case is when the user uncheck a big directory using "choose
what to sync", but would still like to sync a folder within this disabled
tree. The user can now do this with the "add folder" feature
Since 2.3, we even support syncing the same local folder to multiple
remote folder, so why not allow syncing the same remote folder several
times?
Relates to issue #3645
This always returns true since _csync_statedb_is_empty also always
returns true. This function was initially intended to be a shortcut
in case the database doesn't contain any row, but has been broken since
27fb5d9128 and now won't return false
unless the statement fails.
The _last_db_return_error in _csync_detect_update would take care
of that code path in a more direct and clearer way anyway.
This remove the remaining "other" fields of the sync log to save a
bit of memory.
other_etag and other_fileId don't give much information to the users
and other_instruction will always be INST_NONE anyway.
other_modtime and other_size are kept since they are sometimes used.
They were renamed to have a bit more meaningful name.
SyncEngine::checkPermissions will now fetch its information from the
csync trees since they are now preserved until right after this point.
Fixes#3213
Now that csync is using a more convenient data structure for
its file trees, wait a little bit longer before destroying them and
fetch the remote permissions from the remote tree there instead.
Only store the path since they represent the same thing, and do the
phash conversion during DB lookup like done in libsync.
We could get rid of everything since we also have an index on the path
column, but since it's the primary key this makes the migration non-trivial.
Now that they use the same structure, avoid _csync_detect_update
having to recreate another instance and transfer everything manually.
Any instance created during discovery should now be used all the way
up to SyncEngine::treewalkFile.
This also makes sure that the path and types are properly set in that
object instead of having to pass everything as separate parameters.
This gets rid of csync_ftw_flags_e which was now converted from,
and to csync_ftw_type_e, already in the csync_file_stat_t.
Issue #1817
Some filesystems, vms or other limitations make using the WAL journal
mode impossible. We are notified of this problem through an sqlite
IOERR for SHMMAP. In that case We want to attempt to fall back to the
DELETE journal mode.
The query args of POST requests become the request body. If there's a
redirect, the redirected url will therefore not contain the query
arguments. Use an explicit request body to make the redirection work.
Otherwise adding patterns that start with # are impossible to add, since
they get treated as comments. Also add this escaping for patterns added
in the ui.
Also add logging of extended error codes for this IO error, maybe we can
become more specific about which situations should trigger a journal
mode switch.
This is the first time the account url may update outside of
account setup.
Summary of redirection handling:
1. During account setup (wizard)
- status.php gets permanently redirected -> adjust url
- authed PROPFIND gets *any* redirection -> adjust url
2. During connectivity ping (ConnectionValidator)
- status.php gets permanently redirected -> adjust url (new!)
All other redirections should be followed transparently and
don't update the account url in the settings.
When created from scratch, the owncloud icon won't be picked-up by
explorer unless the right attributes are set on both the sync root and the
Desktop.ini file.
Issue #2446
When the GET request from askFromUser is scheduled on the QNAM inside
the slot that handles the QNetworkReply::finished signal, it seems to
not get processed at all.
This workaround moves the sending of the new GET to the event loop,
sidestepping the problem.
Merge csync_create and csync_init into the constructor and
replace csync_destroy with the destructor.
Also use a QByteArray for csync_s::root_perms and flatten
csync_rename_s as a rename sub-struct of csync_s since it
can now handle C++ types.
The only difference with csync_s::current is that it's
assigned the value of csync_s::local::type and
csync_s::remote::type, which never change. So might as
well only use the "current" field with constants.
Just expose csync_file_stat_t since we don't need an abstraction layer
anymore. Also pass the nodes of both trees directly to the visitor
function.
Issue #1817
Also move csync_normalize_etag to common/utility since we
don't need the char* function anymore.
Remove the single space file_stat->remotePerm codepath since
this won't be used in csync anymore since
8de3bda0b1.
Issue #1817
This is the first commit trying to unify csync_file_stat_s,
csync_vio_file_stat_s and csync_tree_walk_file_s. Use QByteArray
and unique_ptr already since I'm not used to track memory allocations
and this will make the transition easier.
Issue #1817
Now that csync builds as C++, this will avoid having to implement
functionalities needed by csync mandatorily in csync itself.
This library is built as part of libocsync and symbols exported
through it.
This requires a relicense of Utility as LGPL. All classes moved into
this library from src/libsync will need to be relicensed as well.
This will allow us to unify data structures between csync and libsync.
Utility functions like csync_time and c_std are still compiled as C
since we won't need to be coupled with Qt in the short term.
Comparison of file sizes for potential conflicts was added in
0eb9401c62, but did not extend to checking
the file size in case of potential local moves.
This commit adds this check and adds tests for various move+change
scenarios.
On Mac, this halves the time spent in csync_excluded_traversal
when using check_csync_excluded_performance. A similar performance
increase is seen on linux.
Fix performence regression from commit d66c2b5fae
For every new file we would look up every parent directories. Allocating
a new QByteArreay for every parent riectory just to know if it is in the other
tree is wasting lots of CPU.
Use a ByteArrayRef trick, similar to QStringRef
To reproduce, log in and click "authorize" on the browser, then close
the browser before the client has replied, (but after redirected to localhost,
i.e. when the client is asking the server for the token)
The problem is that socket can be destroyed so we don't need to answer on a
destroyed socket.
This gets rid of the csync_statedb sqlite layer and use
the same code and same connection as the rest of the SyncEngine.
Missing functions are added to SyncJournalDb and change a few minor
things (like changing SyncJournalFileRecord::_modtime to be an int64
instead of a QDateTime, like it was in csync).
The current implementation would return the same value whether the query failed
or if no row would be found. This is something that is currently checked by csync
and needs to be provided if we want to use SyncJournalDB there.
Adjusted all call sites to also check the return value even though they
could still just rely on rec.isValid(), but makes it more explicit as to what
happens for database errors in those cases, if we ever want to gracefully handle
them.
We need to use concatPath to avoid possible double '/' in the URLs if the
account url() ends with '/'.
This has become even more of a problem since commit
d1b8370a4a which was resolving the url after
a redirect where most server actually add a '/' if the url is a folder
For the remote this was already done long time ago
For windows, this was already partially done
The goal is to avoid needless conversion of the path to local encoding.
Create a specific type that parses the permissions so we can store
it in a short rather than in a QByteArray
Note: in RemotePermissions::toString, we make sure the string is not
empty by adding a space, this was already existing before commit
e8f7adc7ca where it was removed by mistake.
Some slot were protected or private but needed to be public.
Some needed a static_cast (can't use qOverload because it is in Qt 5.7)
This is not only a partial change.
This is motivated by the fact that QMetaObject::noralizeSignature takes 7.35%
CPU of the LargeSyncBench. (Mostly from ABstractNetworkJob::setupConnections and
PropagateUploadFileV1::startNextChunk). It could be fixed by using normalized
signature in the connection statement, but i tought it was a good oportunity
to modernize the code.
This commit only contains calls that were automatically converted with clazy.
Set OWNCLOUD_UPLOAD_CONFLICT_FILES=1 to trigger this behavior.
Note that this is experimental and unsupported. The real feature is
likely to end up in 2.5.
Uploading conflict files is simply done by removing the pattern from
csync_exclude. The rest here deals with making the conflict notification
ui approximately work.
There are still some concerns about where an uploaded conflict file
appears in the sync protocol and issues list (it should be in both, but
is only in one of them currently!).
See #4557.
* The sharing ui does a propfind anyway: use that to query the new
property as well!
* For the socket api, asynchronously query the server for the right url
when an action that needs it is triggered.
The old, manually generated URL will be used as fallback in case the
server doesn't support the new property or the property can't be
retrieved for some reason.
Depends on owncloud/core#29021
... or child folders
There is also no real reason to forbid the user from syncing the same
folder to multiple location on its hardrive.
A real use case is when the user uncheck a big directory using "choose
what to sync", but would still like to sync a folder within this disabled
tree. The user can now do this with the "add folder" feature
Since 2.3, we even support syncing the same local folder to multiple
remote folder, so why not allow syncing the same remote folder several
times?
Relates to issue #3645
This always returns true since _csync_statedb_is_empty also always
returns true. This function was initially intended to be a shortcut
in case the database doesn't contain any row, but has been broken since
27fb5d9128 and now won't return false
unless the statement fails.
The _last_db_return_error in _csync_detect_update would take care
of that code path in a more direct and clearer way anyway.
This remove the remaining "other" fields of the sync log to save a
bit of memory.
other_etag and other_fileId don't give much information to the users
and other_instruction will always be INST_NONE anyway.
other_modtime and other_size are kept since they are sometimes used.
They were renamed to have a bit more meaningful name.
SyncEngine::checkPermissions will now fetch its information from the
csync trees since they are now preserved until right after this point.
Fixes#3213
Now that csync is using a more convenient data structure for
its file trees, wait a little bit longer before destroying them and
fetch the remote permissions from the remote tree there instead.
Only store the path since they represent the same thing, and do the
phash conversion during DB lookup like done in libsync.
We could get rid of everything since we also have an index on the path
column, but since it's the primary key this makes the migration non-trivial.
Now that they use the same structure, avoid _csync_detect_update
having to recreate another instance and transfer everything manually.
Any instance created during discovery should now be used all the way
up to SyncEngine::treewalkFile.
This also makes sure that the path and types are properly set in that
object instead of having to pass everything as separate parameters.
This gets rid of csync_ftw_flags_e which was now converted from,
and to csync_ftw_type_e, already in the csync_file_stat_t.
Issue #1817
Some filesystems, vms or other limitations make using the WAL journal
mode impossible. We are notified of this problem through an sqlite
IOERR for SHMMAP. In that case We want to attempt to fall back to the
DELETE journal mode.
The query args of POST requests become the request body. If there's a
redirect, the redirected url will therefore not contain the query
arguments. Use an explicit request body to make the redirection work.
Otherwise adding patterns that start with # are impossible to add, since
they get treated as comments. Also add this escaping for patterns added
in the ui.
Also add logging of extended error codes for this IO error, maybe we can
become more specific about which situations should trigger a journal
mode switch.
This is the first time the account url may update outside of
account setup.
Summary of redirection handling:
1. During account setup (wizard)
- status.php gets permanently redirected -> adjust url
- authed PROPFIND gets *any* redirection -> adjust url
2. During connectivity ping (ConnectionValidator)
- status.php gets permanently redirected -> adjust url (new!)
All other redirections should be followed transparently and
don't update the account url in the settings.
When created from scratch, the owncloud icon won't be picked-up by
explorer unless the right attributes are set on both the sync root and the
Desktop.ini file.
Issue #2446
When the GET request from askFromUser is scheduled on the QNAM inside
the slot that handles the QNetworkReply::finished signal, it seems to
not get processed at all.
This workaround moves the sending of the new GET to the event loop,
sidestepping the problem.
Merge csync_create and csync_init into the constructor and
replace csync_destroy with the destructor.
Also use a QByteArray for csync_s::root_perms and flatten
csync_rename_s as a rename sub-struct of csync_s since it
can now handle C++ types.
The only difference with csync_s::current is that it's
assigned the value of csync_s::local::type and
csync_s::remote::type, which never change. So might as
well only use the "current" field with constants.
Just expose csync_file_stat_t since we don't need an abstraction layer
anymore. Also pass the nodes of both trees directly to the visitor
function.
Issue #1817
Also move csync_normalize_etag to common/utility since we
don't need the char* function anymore.
Remove the single space file_stat->remotePerm codepath since
this won't be used in csync anymore since
8de3bda0b1.
Issue #1817
This is the first commit trying to unify csync_file_stat_s,
csync_vio_file_stat_s and csync_tree_walk_file_s. Use QByteArray
and unique_ptr already since I'm not used to track memory allocations
and this will make the transition easier.
Issue #1817
Now that csync builds as C++, this will avoid having to implement
functionalities needed by csync mandatorily in csync itself.
This library is built as part of libocsync and symbols exported
through it.
This requires a relicense of Utility as LGPL. All classes moved into
this library from src/libsync will need to be relicensed as well.
This will allow us to unify data structures between csync and libsync.
Utility functions like csync_time and c_std are still compiled as C
since we won't need to be coupled with Qt in the short term.
By setting the icon in Desktop.ini of the root folder, this adds the icon
both when browsing the folder directly and to the sidebar shortcut.
To avoid overwriting any user setting that could exist in Desktop.ini,
only do this if the file doesn't exist. Editing .ini files on Windows
isn't trivial and isn't worth it given that this file won't exist most
of the time.
Fixes#2446
The Job is a LsColJob, not a MkColJob!
Reproduce by entering a name with invalid character that cause an error 400
in the folder wizard's remote path line edit.
(Relates issue #5924)
The mac style has the concept of Layout Item Rectangle that bleeds
widgets margins into parent margins. This unfortunately doesn't work
when the parent layout doesn't have any margin, like we do when we
have the hierarchy:
Vertical(normal margin)
/ Horizontal(no margin, uses parent layout spacing)
/Widget
I guess Qt should propagate remaining uneaten margins to grand-parent
layouts and so on to have this work properly, but nobody seems to
have touched that code since Qt 4.4.
So just try to make the problem less worse by making sure that
all checkboxes we want to align are either in the same layout, or
in a loyout of their own.
Issue #5492
Allow upgrade path when the server removes support for oauth
Relates: https://github.com/owncloud/client/issues/5848#issuecomment-317353049
We also need to force the account to commit the config to the disk,
otherwise we may not register we are no longer using owncloud and we
risk sending the password as the token to the token refresh API call
Before commit d3b00532b1,
fetchFromKeychain was called everytime we detect that the creds are
invalid (in AccountState::slotInvalidCredentials)
But since that commit, AccountState was calling askFromUser directly,
breaking the refresh of the token.
So I made sure AccountState::slotInvalidCredentials still calls
refreshAccessToken.
Another change that was made was too be sure to clear the cookies
in HttpCredentials::invalidateToken even when we are only clearing the
access_token. That's because the session with a cookie may stay valid
longer than the access_token
We only want to know if they were touched within the last 15 seconds,
so change the data structure to use a QMultiMap, and sort them by
QElapsedTimer. This allows us to iterate over old entries ordered by
time and to stop once we find a recent entry.
This makes the look-up slower but in most cases the folder watcher
will report any change within milliseconds, and we start from the
most recent. What this really makes slower are actual user file
changes while a fast sync is underways which will need to iterate
over the whole map to find out the file isn't there.
This reduces the growth of the memory usage when downloading a large
amount of files.
We need Qt 5.9 for HTTP2 because, even if Qt 5.8 already has support
for it, there is some critical bug in the HTTP2 implementation which
make it unusable [ https://codereview.qt-project.org/186050 and
https://codereview.qt-project.org/186066 ]
When using HTTP2, we can use many more parallel network request, this
is especially good for small file handling
Lower the priority of the GET and PUT propagation jobs, so the quota
or selective sync ui PROPFIND will not be blocked by them
Since the user is already in the browser, put the error message in the
browser with a message to log out and then log in as the right user.
Issue #5895
Issues:
- #5661 On mac, iconv did not support all of unicode and some
files with emoji in the filename could not be uploaded
- #5719 , #5676 On linux, we will now support non utf-8 locale
Since these errors are blacklisted, it can take up to 24h to retry items
that had a 507 error for a while. This way users can intervene and cause
an upload attempt immediately.
It now produces a summary error message indicating the problem.
Adjust blacklist database table to contain 'errorCategory'. This is
useful for two things:
- Reestablishing summary messages based on blacklisted errors. For
example if we don't retry a 507ed file, we still want to show the
message about space on the server
- Selectively wiping the blacklist: When we have ui for something like
"I deleted some files, please retry all files now!", we want to
delete all blacklist entries of a specific category only.
* A bunch of code was determining sync status by ad-hoc comparing some
progress info fields. It can now just check the status, making it
easier to comprehend.
* There's a clear indication for "a new sync is starting", which helps
wiping the issues tab at the right time.
For now we use them for:
* csync errors: This allows them to appear in the sync issues tab
* insufficient local disk space, as a summary of individual file errors
Insufficient remote space will use them too, as might other issues that
are bigger than a single sync item.
Requires https://github.com/owncloud/oauth2/pull/45
This commit moves the reply after we got the token reply from
the server, that allows to reply with an error to the browser
if the login does not work.
The QNAM may continue to outlive both.
Rename Credentials::getQNAM() to createQNAM() while we're at it - it's
used to make a new QNAM that will subsequently be owned by the Account
object.
See d01065b9a1 for rationale.
Relates to
d40c56eda5147cf798a6
It would have been much nicer to keep the menu assigned to the
QToolButton, but if one switches away from InstantPopup (to adjust the
entries before they're displayed), the button always gets a menu
indicator that can't be removed.
Having to update the plugins to add a submenu in the context menu,
it's more cost-effective to remove support for legacy macOS versions
than implementing and testing this setup.
The Windows shell extension relied on the response of
SHARE_MENU_TITLE to advance its state machine, but in order
to use the new GET_STRINGS instead, we need to know when the
last string was received. Also add BEGIN for consistency.
* SocketAPI has COPL_LOCAL_LINK / EMAIL_LOCAL_LINK commands
* The nautilus and dolphing shell integrations show a submenu from which
one can share as well as access the private link.
* The SocketAPI provides a new GET_STRINGS command to access localized
strings.
* The private link can also be accessed from the user/group sharing
dialog.
* The numeric file id is extracted from the full id to create the
private link url.
Calling forgetSensitiveData() on account deletion leads to a timer for
clearQNAMCache() being queued. Then the Account object is deleted. The
Credentials object stays alive for now because it has a deleteLater
deleter.
If the timer calls into a slot on the Credentials object, the _account
pointer will be invalid at this time.
As a workaround, move the target slot to Account - that way it will not
be called as the account object is already destroyed.
However since Account and Credentials are mutually dependent, it would
be much preferable if their lifetimes were linked, avoiding this
category of bugs.
The current behavior was introduced in
d40c56eda5 and I currently don't
understand why - maybe there's another way of dealing with the problem
that existed then.
* Add a more functional error view #5516
* Allow filtering of ignores and warnings to see only important bits.
* Navigate from the folder view to the error view by clicking on the
error list with the red background.
* Move the error list into its own ui file to allow easier extension.
* Fix issue around tab id handling in ActivitySettings.
* Rename "Action" column to "Issue".
* Change mouse cursor to hand over button and new error list area
Several OSX fixes provided by guruz.
Before, blacklisted errors were set to FileIgnored status and hence
displayed as warnings. Now, they have their own BlacklistedError
category which allows them to appear as errors in the issues list and in
the shell integration icons.
src/gui/main.cpp:112:9: warning: bool literal returned from 'main' [-Wmain]
Used 1 to keep previous behaviour. I supposed the code was meant
to return success (0), but it does not really matter anyway.
When synchronizing a folder on a samba share, creating files that begin
with ._ is often forbidden. This prevented the client from creating
its ._sync_abcdef.db file.
Now, it'll check whether the preferred filename is creatable, and if
it isn't it'll use .sync_abcdef.db instead.
The disadvantage is that this alternative path won't be ignored by
older clients - that was the reason for the ._ prefix.
* For conflicts where mtime and size are identical:
a) If there's no remote checksum, skip (unchanged)
b) If there's a remote checksum that's a useful hash, create a
PropagateDownload job and compute the local hash. If the hashes
are identical, don't download the file and just update metadata.
* Avoid exposing the existence of checksumTypeId beyond the database
layer. This makes handling checksums easier in general because they
can usually be treated as a single blob.
This change was prompted by the difficulty of producing file_stat_t
entries uniformly from PROPFINDs and the database.
- Add category to the all messages (they did not have it was merged right after
the patch to add category everywhere, but this code did not have it.)
- Make sure there is no warnings in the normal flow. (The wizard does a request
without authentication to determine the auth type)
All our crypto code is handled by qt nodaways.
No need to carry this dependency.
Especially since it causes warnings on system where there are
twp openssl version installed:
/usr/bin/ld: warning: libcrypto.so.1.0.0, needed by /usr/lib/libQt5Network.so.5.9.0, may conflict with libcrypto.so.1.1
Issue #5783
When the directry that should be removed by selective sync contains changes,
we ignore the whole sub tree instead of only ignoreing new files.
We cannot ignore the whole directory, we need to ignore only the directory
that do not have files to remove
See owncloud/enterprise#1966
If the server and the client's database go out of sync, there could be
persistent 404 errors. This change ensures that the problem corrects
itself eventually by triggering a remote discovery of the file's
parent folders.
It does not address the root cause that might have lead to the
divergence.
Older ownCloud servers like 8.2.0 used to use integer ids.
This broke when switching to QJson because toString() on JSON
values is strict and returns "" for integers.
This should help clearly delimiting them in the log for multi-account setups.
This information is already available elsewhere in the log in any case.
Issue #5672
These would otherwise be line-wrapped by clang-format,
and then consecutive reformattings remove the aligned
comment indentation
Example:
int a; // too long comment
->
int a; // too long
// comment
->
int a; // too long
// comment
When a new folder becomes selective-sync excluded, we already mark it
and all its parent folders with _invalid_ etags to force rediscovery.
That's not enough however. Later calls to csync_statedb_get_below_path
could still pull data about the excluded files into the remote tree.
That lead to incorrect behavior, such as uploads happening for folders
that had been explicitly excluded from sync.
To fix the problem, statedb_get_below_path is adjusted to not read the
data about excluded folders from the database.
Currently we can't wipe this data from the database outright because we
need it to determine whether the files in the excluded folder can be
wiped away or not.
See owncloud/enterprise#1965
Use qCInfo for anything that has general value for support and
development. Use qCWarning for any recoverable error and qCCritical
for anything that could result in data loss or would identify a serious
issue with the code.
Issue #5647
This gives more insight about the logs and allow setting fine-tuned
logging rules. The categories are set to only output Info by default
so this allows us to provide more concise logging while keeping the
ability to extract more information for a specific category when
developping or debugging customer issues.
Issue #5647
Add the log level and category name in the output. Only output the
thread ID and function name for qCDebug statements as they are not
necessary for general use and make the log harder to read.
Also make sure that the message pattern is set when NO_MSG_HANDLER is
used. Using an environment variable should have priority over it anyway.
When we first detect a 503 (probably from a PROPFIND) and enter the
ServiceUnavailable state, we new trigger a status.php query that will
switch the state to MaintenanceMode if necessary.
Before this patch, to deep folder would just be ignored, without any feedback.
This patch makes it so deep folder are properly shown as ignored in the UI.
Also increase the MAX_DEPTH
Issue: #1067
I'm confident this is unnecessary. The original bug in #3283 was
to call ignoreSslErrors() without an argument in the 'accept'
case, which meant ignoring *all* subsequent SSL errors.
With that fixed, explicitly aborting the reply and resetting QNAM
is not needed since not ignoring the error will lead to the SSL
handshake failing.
See also:
75b38d1a2f (workaround introduced)
89376e14d6 (real fix)
76ce5adbf0 (cherry-pick of workaround)
* Allow creating nameless shares
* Display token as name for nameless shares
(both to be consistent with server)
* Allow changing a share's name by editing it in the table
* Minor adjustments
Starting from oC 10.0.0 having several public link shares with
different attributes for a path will be supported. This adds
functionality to create, edit and delete these public link shares.
The behavior is currently gated by server version, but should be
adapted to use a capability as soon as one is introduced, see
owncloud/core#27622.
The UI reduces to a single-share version when talking to older servers.
Testing scenarios:
* Link sharing is disabled (by capability, not by theme)
* Required passwords
* Required expiry
* Forbidden 'allow upload' for folders
* New and old servers
Newer servers will have the option of hiding version, versionstring, edition
and productname. They will always send the full information in the capabilities.
By default QNetworkReply::errorString() often produces messages like
"Error downloading <url> - server replied: <reason>"
but the "downloading" part invariably confuses people since the
error might very well have been produced by a PUT request.
This commit produces clearer error messages for HTTP errors.
Additionally:
* Remove some unnecessary null checks from slots connected to
network job signals and document that these signals never send
null replies.
* There was a bug where AbstractNetworkJob::_timedout wasn't
set when derived classes overrode slotTimeout. We now ensure
it's always set by disallowing overrides of slotTimeout.
Instead it now calls onTimedOut, which allows custom handling.
* Several subclasses declared errorString, isTimedOut. Move
these to AbstractNetworkJob.
* Unify handling of OC-ErrorString (via the new, general
Job::errorString)
* Add documentation in various places.
Before we would only open the part of the url that the users entered.
Now if the wizardUrlPostfix is used this is should be appended to that
when opening the browser on the result page.
Backtrace looks like this:
Crash: EXCEPTION_ACCESS_VIOLATION_READ at 0x0
File "propagatedownload.cpp", line 234, in OCC::GETFileJob::slotReadyRead
File "moc_propagatedownl_CA5CFSHZDTX34X.cpp", line 86, in OCC::GETFileJob::qt_static_metacall
File "qobject.cpp", line 495, in QMetaCallEvent::placeMetaCall
File "qobject.cpp", line 1256, in QObject::event
File "qapplication.cpp", line 3804, in QApplicationPrivate::notify_helper
GETFileJob::slotReadyRead can be called with a QueuedConnection when the
bendwith manager is involved. In that case, if the QNAM was reset
in between, the reply might have been destroyed.
(This is only speculation based on the backtrace)
Backtrace from the crash reporter:
Crash: EXCEPTION_ACCESS_VIOLATION_READ at 0x21
File "qcoreapplication.cpp", line 1281, in QCoreApplication::postEvent
File "qobject.cpp", line 2125, in QObject::deleteLater
File "connectionvalidator.cpp", line 240, in OCC::ConnectionValidator::reportResult
File "connectionvalidator.cpp", line 206, in OCC::ConnectionValidator::slotAuthFailed
File "moc_connectionvalidator.cpp", line 127, in OCC::ConnectionValidator::qt_static_metacall
File "qobject.cpp", line 3716, in QMetaObject::activate
File "moc_networkjobs.cpp", line 653, in OCC::PropfindJob::finishedWithError
File "networkjobs.cpp", line 570, in OCC::PropfindJob::finished
I believe the problem is caused because 'this' was deleted in ConnectionValidator::reportResult
as the signal connectionResult gets emited. The AccountState::slotConnectionValidatorResult
slot does indeed call slotInvalidCredentials which might call {Shibboleth,Http}Credentials::fetchFromKeychain
which might emit fetched directly, which will call AccountState::slotCredentialsFetched
which deletes the _connectionValidator
So use deleteLater when deleting the _connectionValidator, hoping this helps
* make target duration a client option instead of a capability
* simplify algorithm for determining chunk size significantly
* preserve chunk size for the whole propagation, not just per upload
* move options to SyncOptions to avoid depending on ConfigFile
in the propagator
* move chunk-size adjustment to after a chunk finishes, not when
a new chunk starts
Previously the check box was checked if *any* of its sub-permissions
were granted. This can hide the fact that only a limited subset of them
are actually granted.
The new behavior is to display as "partially checked" if only some
of the sub-permissions are available. Clicking the check box itself
still toggles between granting all or none of them.
When there is no errors _syncResult.firstItemError() is NULL, and accessing
it's _file member is an undefined behavior. (Thankfully, createGuiLog did not
use the string when the count was 0, but we are not supposed to create
null references.
Found with the UB sanitizer:
src/gui/folder.cpp:348:49: runtime error: member access within null pointer of type 'OCC::SyncFileItem'
src/gui/folder.cpp:348:19: runtime error: reference binding to null pointer of type 'const QString'
The backtrace seems to indicate that the account is invalid.
I don't know how this can happen, maybe the account's display
name was changed while the app is running?
Backtrace:
Crash: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS at 0x18
Module "owncloud", in OCC::AccountState::account
Module "owncloud", in OCC::ActivityListModel::data
Module "owncloud", in OCC::ActivityItemDelegate::paint
Module "QtWidgets", in QListView::paintEvent
Module "QtWidgets", in QWidget::event
Module "QtWidgets", in QFrame::event
Module "QtWidgets", in QAbstractScrollArea::viewportEvent
Module "QtWidgets", in QAbstractItemView::viewportEvent
Module "QtWidgets", in QAbstractScrollAreaFilter::eventFilter
Module "QtCore", in QCoreApplicationPrivate::sendThroughObjectEventFilters
The destructor of the PropagateItemJob will access the propagator's
_activeJobList. So the _rootJob needs to be destroyed before it.
Order of destruction is the reverse of the order of the members in
the class. So put it at the end so it can be destroyed first.
(This made TestSyncEngine::testDirDownloadWithError crash sometimes
in the master branch)
I didn't want to touch this visible translated string in a
patch release so I just switched the data.
For Qt versions before 5, we continue to show the build-time version
of OpenSSL but it's clearly marked as such.
In 8ef11a38c9, we started blacklisting
SoftError for 0 seconds. But if the two sync happen with less than
1s interval, we would still prevent them to happen.
So make sure we expire if 0 seconds have expired
The backtrace looks like:
File "atomic_base.h", line 396, in QString::~QString
File "qlist.h", line 442, in OCC::FolderStatusModel::slotUpdateDirectories
This is the only QList operation, and it may crash if the list is empty.
It can be empty if the propfind returned empty results.
I'm not sure how this can be possible to have an empty list there since
the server is always supposed to return at least one entry, for the directory
itself. But it can happen if a directory was transformed in a file, or
if there is a bug on the server.
This is a possible fix for #3850, applying custom window flags (http://doc.qt.io/qt-5.8/qt.html#WindowType-enum) on the connection method dialog. These avoid resizing the dialog and displaying a maximize button on OS X and context help button on Windows.
It is possible to create files with filenames that differ
only by case in NTFS, but most operations such as stat and
open only target one of these by default.
When that happens, we want to avoid uploading incorrect data
and give up on the file.
Typically this situation should never occurr during normal use
of Windows. It can happen, however, when a NTFS partition is
mounted in another OS.
So the title of the column is fully visible.
Adjust the default size and the size policy of the label so it behave
correctly as the window get resized
Issue #5600
The crash reporter shows many crashes in OwncloudPropagator::scheduleNextJob.
We don't really know what could be the cause, but it's probably because
the _activeJobList contains dangling pointer.
So this patch makes sure to remove all the jobs from this list as they get
destroyed.
* For requests:
- reuse the original QNetworkRequest, so headers and attributes
are the same as in the original request
- determine the original http method from the reply and the request
attributes
- keep the original request body around such that it can be sent
again in case the request is redirected
* Simplify the interface that is used for creating new requests in
AbstractNetworkJob.
Backtrace from the crash reporter:
Crash: EXCEPTION_ACCESS_VIOLATION_READ at 0x401
File "moc_activitylistmo_M5OEXJ7XGJYTWT.cpp", line 92, in OCC::ActivityListModel::qt_static_metacall
File "qobject.cpp", line 3730, in QMetaObject::activate
File "moc_networkjobs_7AMNCW4BBANVRK.cpp", line 1342, in OCC::JsonApiJob::jsonReceived
File "networkjobs.cpp", line 714, in OCC::JsonApiJob::finished
File "abstractnetworkjob.cpp", line 207, in OCC::AbstractNetworkJob::slotFinished
File "moc_abstractnetwor_PFI2TXGQHRE33H.cpp", line 98, in OCC::AbstractNetworkJob::qt_static_metacall
File "qobject.cpp", line 3730, in QMetaObject::activate
File "moc_qnetworkreply.cpp", line 367, in QNetworkReply::finished
File "qnetworkreplyhttpimpl.cpp", line 2100, in QNetworkReplyHttpImplPrivate::finished
File "qnetworkreplyhttpimpl.cpp", line 279, in QNetworkReplyHttpImpl::abort
My theory is that the AccountState stored in a property of the job was destroyed.
before the job timed out.
Therefore, the qobject_cast within the qvariant_cast would call the metaObject()
virtual function on a dangling pointer.
Fix it by storing a QPointer instead which will track the deletion.
This leads to crashes since we changed the connection to the parent
jobs not to be queued anymore.
We don't really need to bubble up the finished state through
parents in that case, and it would also mean that we'd recurse
all the way through leaves as we go up to each parent. So just call
abort directly on the OwncloudPropagator and make sure the abortion
call is posted to the event loop.
Avoid using connections to report up the job tree for signals
that we can directly communicate to the OwncloudPropagator.
This slightly reduces the memory usage and avoid passing those calls
through the whole parent chain.
In preparation for the PropagateDirectory refactoring, simplify things
by removing WaitForFinishedInParentDirectory, which is currently
implemented as a one-level check.
This value is important for directory items, but is however never
used since a directory CSYNC_INSTRUCTION_RENAME item will always be in
PropagateDirectory::_firstJob, which will have to pass through its own
PropagateDirectory job's parallelism() before reaching the parent's
_subJobs optimization.
Since PropagateDirectory::parallelism can only return WaitForFinished
or FullParallelism, that value is lost. So this commit doesn't
change the behavior for directories, and allow file renames to be
scheduled in parallel across directories (which isn't a problem).
It could be possible that _firstJob is marked as finished if
aborted before its parent PropagateDirectory was marked as finished,
allowing a posted scheduleNextJob call to schedule the child job
in-between.
- I checked every occurence of a '%2' and make correct use of the
QString::arg overload that takes several argument instead of chaining
them, because the first argument can contains a '%1'
- I tried to look for every label that they either use plain text or richtext
and escape the user provided strings in there.
By default, followRedirects is true for all requests, to transparently
handle redirections. In the wizard, we have special redirect-handling
code though and that was being skipped.
Setting the flag to false allows the wizard to be aware of redirects
and to handle them in the correct way. Tested with the server described
in
https://github.com/owncloud/administration/tree/master/redirectServer
There's a second bug here, where followRedirects always converts
redirected requests to the GET verb. That means redirected PROPFINDs
will never have worked. This change un-breaks them for the wizard only.
There should be no case that previously worked that stops working now.
The test sets OWNCLOUD_MAX_PARALLEL to 1 to disable parallelism.
But since the max amount of parallelism is twice as much, that does not
work.
So change the way we compute the hardMaximumActiveJob: Use the value of
OWNCLOUD_MAX_PARALLEL to maximize this amount and base the maximum amount
of transfer jobs on it instead of the other way.
A result of this change is that, in case of bandwidth limit, we keep the
default of 6 non-transfer jobs in parallel. I believe that's fine since
the short jobs do not really use bandwidth, so we can still keep the same
amount of small jobs.
This reverts commit e1f5a49c21.
Retrying uploads with insufficent storage errors frequently leads to
high server traffic. See #5537 for links and a sketch of a correct
solution.
If we call
setConfiguration(QNetworkConfiguration());
This sets an invalid configuration on the QNAM.
But later, when we really go online because interfaces are discovered,
QNetworkAccessManagerPrivate::_q_onlineStateChanged is called (with isOnline=true).
And this will set the state to disconnected because customNetworkConfiguration is
true, and the networkConfiguration state is disabled.
The workaround we to fix another bug on Windows in which the default network
configuration was not behaving properly.
The issue on linux is hard to reproduce and only happen in some condition,
but it was reproduced on smashbox when they run two owncloudcmd at the same time.
Issues: #4720 , #3600
We were removing the wholme journal db when the user wanted to keep all files,
But that would also remove the selective sync lists.
We should only remove the metadata table.
Issue #5484
- Put all tests in the bin directory so that DLLs can be loaded
- Add missing exports
- Skip tests that use code depending on zlib
- The "GMT" timezone is named differently, use the int constructor instead
5 tests are still failing, it's not really worth fixing at the moment
since no developper is currently using Windows as its main platform.
It could be possible that _firstJob is marked as finished if
aborted before its parent PropagateDirectory was marked as finished,
allowing a posted scheduleNextJob call to schedule the child job
in-between.
Previously, we'd try migrating from legacy settings if reading
the settings failed with an error. Now, we try again after a
couple of seconds and eventually give up.
We can do that because the only changes that were in master but not in 2.3 were the
translations change and documentation change, and the support for the 'M' permission
which we want in 2.3.
The sync engine rely on the 'M' in premission to ask for confirmation
(As requested in issue #5340)
But we only want to ask the premission for the 'root' of the mounting point and not
for every subfolders within it.
So we change the discovery phase in a way that it does not keep the 'M' for
children within the external storage.
Added two checkboxes in the Account Wizard in the advanced page to change the first options.
Also added a checkbox in the general settings to ask for confirmation for external storages.
Theme options allow to hide the checkboxes in the wizard.
As described in issue #5340
We now delete subjobs as their propagation is complete. This allows us
to also release the item by making sure that nothing else is holding a
reference to it.
Remove the stored SyncFileItemVector from SyncEngine and SyncResult
and instead gather the needed info progressively as each itemCompleted
signal is emitted.
This frees some holes on the heap as propagation goes, allowing many
memory allocations without the need of requesting more virtual memory
from the OS, preventing the memory usage from increasingly growing.
This was to catch duplicate emissions for PropagateDirectory but we
don't emit this signal anymore from there.
This fixes a warning about PropagatorJob not being a registered metatype.
This reverts commit fe42c1a818.
Previously if you paused/unpaused a folder for a disconnected account
they would prepare to sync and thus display the 'Waiting...' text. With
this change, folders that can't possibly sync don't show text like
this.
When account connectivity changes, all unpaused folders will be
scheduled anyway.
The avatar image is fetched from the server async, thus connect a signal
from the account if the avatar changes.
Server feature https://github.com/owncloud/core/pull/26872 is needed.
The avatar pixmap is fetched from the server by the Connectionvalidator,
once it has validated the user name, it queries the avatar pixmap.
If the server does not have the avatar route, an empty pixmap is stored.
Stale chunks might be there because a file was removed or would just not
be uploaded, for any reason.
We just start the DeleteJob but we don't care if it success or not.
Relates to https://github.com/owncloud/core/issues/26981
One of the test is testing the case where the file is modified on the server
during the upload. So this test the precondition failed error.
The FakeGetReply logic was modified because resizing a 150MB big QByteArray
by increment of 16k just did not scale when downloading a big file.
Relates to https://github.com/owncloud/core/issues/26981
We do not track the success or error of the DeleteJob because it does not
matter. If it fails, it might be because the chunks were already removed.
If not, the chunks will be stale, but the server must anyway do a few
cleanup from time to time because we do not always remove the chunks
The current logic tried to avoid a DB lookup just to fetch whether
the file is shared or not since that info is already in the
SyncFileItem. The implementation would however need to decrease the
sync count for itself (and parents) before emitting the new status,
thus emitting the OK status for parents before that last child that
ended the propagation for that folder.
Change the implementation to achieve what we want: give the
possibility to decSyncCount to use a pre-fetched sharing state while
still doing the emission for all involved files. This ensures that
the leaf file also gets its status emitted before its parents.
Issue #4797
We currently push the SYNC status for all files that will be propagated,
and then the OK status when those files are propagated.
On top of this, we send those statuses to all clients connected, even
if the socket is kept open by an application that only needed to show
a file open dialog. On macOS we're also using an NSConnection which
means that we have to wait for the RPC call to return from the
extension, which makes bulk status changes possibly heavy.
Reduce the time spent needlessly sending status pushes by limiting
them to files requested through that socket since it connected.
To limit the data to store, only remember the parent directory of
files requested, and store those in a bloom filter.
Note that this adds a requirement to shell extensions: they should
make sure that the status cache only contains entries that have been
requested through the socket API. In other words, the status cache
must be empty when each socket client connects to the socket API.
Otherwise the cached icon type will be shown to the user, and the
SocketAPI won't push new status for that file if it didn't receive
a RETRIEVE_FILE_STATUS.
- Use the looked-up method index also for the invocation
- Do the method name concatenation already on QByteArray since we'll
convert anyway
- Use staticMetaObject instead of metaObject()
Shrinks owncloud binary by 24 KB and libowncloudsync by 14 KB.
I don't know if it has influence on memory usage or runtime speed though.
Was worth a try.
Previously this wasn't happening for errors that were not
NormalErrors because they don't end up in the blacklist.
This revises the resetting logic to be independent of the
error blacklist and make use of UploadInfo::errorCount
instead.
412 errors should reset chunked uploads because they might be
indicative of a checksum error.
Additionally, server bugs might require that additional
errors cause an upload reset. To allow that, a new capability
is added that can be used to advise the client about this.
This displayName() seemed to be based on Account::user() which used
to call _credentials->user(). But then we repurposed user() to be
davUser() and this usage wasn't updated to point back to the username
used for the credentials.
Some custom server use persistent cookies with the auth token. So we should
clear all the cookies when disconnecting.
Account::clearCookieJar is only called from the HTTPCredentials. This funciton
is not used for shibboleth.
There is probably no reasons to keep the HTTP cookie anyway.
Issue #5370
The re-enables the UI, uses Qt API for importing and
stores the certificate/key in the system keychain.
People who had set up client certs need to re-setup the account. This is ok
since it was an undocumented feature anyway.
Fixes#5408#5407.
The problem was that cleanup of the credentials page set the
credentials of the account back to dummy, thereby overriding
things like shib usernames.
This should be broken since a932eac832.
Otherwise it might happen that the model is inconsistant and this can
lead to crash in the worst case.
(For example, if there was a "fetching" label, and we hide it because it
was a 404. In this case, we would not call begin/endRemoveRows, so the
view could still call the model with an index of row 0, that used to be
for the label, but now correspond to the first element of _subs. And
because _subs is empty, this could lead to crashes)
This could make sure that the network job gets deleted if the parent job gets
deleted, and would avoid crashes like:
Crash: EXCEPTION_ACCESS_VIOLATION_READ at 0xffffffff8b008a04
File "qiodevice.cpp", line 1617, in QIODevice::errorString
File "propagatedownload.cpp", line 264, in OCC::GETFileJob::slotReadyRead
File "moc_propagatedownload.cpp", line 85, in OCC::GETFileJob::qt_static_metacall
File "qobject.cpp", line 3716, in QMetaObject::activate
File "moc_qiodevice.cpp", line 154, in QIODevice::readyRead
File "qnetworkreplyhttpimpl.cpp", line 1045, in QNetworkReplyHttpImplPrivate::replyDownloadData
(#5329)
The csync log level was only set up on startup, and for log files.
Fix the issue by making Logger::isNoop rely on being explicitly activated
for the log window instead of relying on the presence of a connected
signal, and move the csync log level logic in Logger.
The compiler seems to use signed enums and we need to reserve an extra
bit for the sign to avoid the 2 value to overflow and being interpreted
as -2 when read, and thus not being correctly compared to the full enum
value.
The rules to select the webdav url are now:
- If the server reports that the new chunking algorithm is working,
always use remote.php/dav/files/<username>
This capability can be overriden with an environment variable
- Otherwise, use the dav path provided by the theme, which defaults to
remote.php/webdav
This means that with the newer server, the branding can no longer override
the webdav URL. If there is still an usecase for the branding to do so, we
need to find another way to override it. But it is now more complicated to
configure as might need include the username and need different endpoint
depending on the operations (chunks or not)
Issue #4007
As the URL might be print on the logs.
Also don't change the scheme from http to owncloud.
This was required before when we were using neon through csync, but now that
we use QNAM for everything we don't need it. The credentials from the account
are used.
We are going to change the webdav path depending on the capabilities.
But the SyncEngine and csync might have been created before the capabilities
are retrieved.
The main raison why we gave the path to the sync engine was to pass it to csync.
But the thing is that csync don't need anymore this url as everything is done by the
discovery classes in libsync that use the network jobs that use the account for the urls.
So csync do not need the remote URI.
shortenFilename in folderstatusmodel.cpp was useless because the string is the
_file of a SyncFileItem which is the relative file name, that name never
starts with owncloud://.
All the csync test creates the folder because csync use to check if the folder
exists. But we don't need to do that anymore
Some listeners detect whether a sync is finished by checking
for isUpdatingEstimates and completedFiles >= totalFiles. But
if a sync didn't transfer any files we never sent signal
with these values. Now we do.
The "S" in the permission is only for the "Shared with me" files.
It is only used to show the shared status in the overlay icons.
But we also wish to show the shared status for files that are shared
"by" the users. We can find that out using the 'share-types' webdav
property. If set, then we are sharing the object.
We fake a 'S' in the permission as for our purpose, they mean the same.
Issue #4788
The ownCloud 9.1 server has a data-fingerprint property that the admin must
change in case of backup restoration. When this change, the client understands
that a backup was restored, and will generate conflict files and re-upload
new files.
The heuristics based system checks that there is at least two files wose mtime
is put back in the past and no files that goes forward. In that case we ask the
user before creating the conflicts.
This commit disable the heuristics for newer server that have the data-fingerpint.
And change the heuristics to two hours because we want to avoid false positive due
to some clock error, and that 2 hours of lost due to backup restoration is probably
not so bad.
We only ask the user in the heuristics based aproach so in practice this mean that
the "backup-detected" dialog will no longer appear with newer server.
Relates issues #5260, #5109
Otherwise progress listeners think it's still the last-completed
item when the next sync starts. This lead to spurious entries in
the "Recent Changes" list.
Update from commit 05ce8a23cdc12e825532dc6de06c267fb8d48b4f from
https://github.com/dragotin/QProgressIndicator
Which itself is forked from commit e5ba0fd09bfd43b067ee3646d70b294c7efcb558 from
upstream, with additional license header.
It was relicensed to MIT according to
14bb9d10e2
Relates to issues #5180 and #5184
Issue #5224
Two problems:
- In the discovery phase, we need to check the selective sync entries of
the source path in case of renames.
- When the rename is done, we need to actually update the black list in the
database.
Some tests (such as FolderManTest) can polute the config file with invalid
accounts.
(That's because most of the code, (even in libsync) always instentiate a ConfigFile)
The crash reporter shows a lot of crashes in sqlite3_clear_bindings
which seems to indicate that _stmt is null. We should guard against
a null value in order to avoid crashing.
This should only happen if the prepare call fails. We don't usually
check the return value of the prepare call, but if _stmt is null, the
exec call should return false, not true. We check the result of the
exec call, so this should then abort the sync with an error, rather
than crashing.
- Use a white icon if the context menu is visible.
- Enable `QIcon::setIsMask` if compiled on Qt >= 5.6 to allow automatic
macOS color handling.
- No changes if the colored icons are used.
One local folder can now be configured as sync target for multiple
accounts as long as their url and user differ.
Also this patch accepts that the sync folder is behind a symlink.
Also this patch fixes a bug that before the user input was taken
canonically which was not working for the symlink handling.
Instead of using the regular selective-sync UI (where it's unclear what
the "Cancel" button would even mean in this context), provide a
different set of buttons that allow the user to quickly synchronize
all pending big folders, none of them, or perform manual changes
as usual.
Tray: Workaround collection
* QDBus workaround for Qt 5.5.0 only, there were reports of the tray
working fine with 5.5.1. #5164
* OWNCLOUD_FORCE_QDBUS_TRAY_WORKAROUND to force the workaround on an off
* OWNCLOUD_TRAY_UPDATE_WHILE_VISIBLE to enable or disable updating of
the menu while it's visible - disable by default due to problems on OSX and Xubuntu.
* Track the visibility of the tray menu with aboutToShow/aboutToHide
only on OSX - the aboutToHide signal doesn't trigger reliably on linux
* Refactor such that setupContextMenu is different from updateContextMenu
* Don't use on-demand updating of the tray menu when the qdbus workaround
is active, instead to occasional (30s) updates of the tray menu.
We ignored csync log, but we also need to silent Qt debug output.
We need to ignore it at the very begining because there might be
qDebug also in account creation.
Issue #5196
Two bugs:
- The change filed are not considered as move, they are re-downloaded
but the old file was not removed from the database. The change in
owncloudpropagator.cpp takes care of removing the old entries.
- Next sync would then remove the file in the server in the old folder
This was not a problem until we start reusing the sync engine, and
that the _renamedFolders map is not cleared. We were before deleting
a non-existing file. But now we delete the actual file.
Also improve the tests to be able to do move on the server.
This include support for file id.
Issue #5192
In case of the root directory, it may happen that the _item
is empty and the _item->_status is NoStatus. But we still need to report
the proper success or error of the whole propagation. We should really
use _hasError for that. However, _hasError is also defined to NoStatus
if there was no error, so in that case we need to set Success.
This fixes the problem in which the data-fingerprint is not saved on the
database because the SyncEngine think that the sync failed. (Issue #5185)
We wwer enot connecting to the right signal from the check server
job, and therefore we were not catching the condition in which the
json was invalid. We would then never terminate the ConnectionValidator job.
Note that instanceNotFound is also emited if there is a network error.
The log looked like this:
10:25:51.247 OCC::CheckServerJob::finished: status.php from server is not valid JSON!
10:25:51.248 OCC::CheckServerJob::finished: status.php returns: QMap() QNetworkReply::NetworkError(NoError) Reply: QNetworkReplyHttpImpl(0x2b6a790)
10:25:51.248 OCC::CheckServerJob::finished: No proper answer on QUrl("http://localhost/~owncloud/status.php")
10:26:23.235 OCC::AccountState::checkConnectivity: ConnectionValidator already running, ignoring "owncloud@localhost"
10:26:55.235 OCC::AccountState::checkConnectivity: ConnectionValidator already running, ignoring "owncloud@localhost"
[...]
- Replace functions that are provided by MinGW with a Win32-based
implementation
- Explicitly export needed symbols from ocsync.dll
- Rename share.h to sharemanager.h since the name clashes with one
of the Windows headers and get included from there
- Remove the timestamp from the fallback csync stderr logging, it's
not used since we always provide a log callback