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

Segfault when clicking on unconfigured SFTP server #6562

Closed
lefherz opened this issue May 30, 2018 · 6 comments
Closed

Segfault when clicking on unconfigured SFTP server #6562

lefherz opened this issue May 30, 2018 · 6 comments
Assignees
@ogoffart
Labels
ReadyToTest QA, please validate the fix/enhancement sev1-critical
Milestone
2.5.0

Comments

@lefherz
Copy link
Contributor

lefherz commented May 30, 2018 •
edited
Loading

Expected behaviour

The ownCloud settings view should display a proper error message for unconfigured SFTP folders.

Actual behaviour

The client segfaults when trying to display the subfolders of the unconfigured SFTP folder.

Steps to reproduce

  1. unproperly configure SFTP external storage
  2. open testpilot
  3. in the account view, click on the triangle left to the SFTP folder
  4. wait for the client to segfault

Server configuration

demo.owncloud.org

Client configuration

Client version:

testpilotcloud version 2.5.0alpha1 (build 9564)
Git revision 107baf2947959cd61f45951f7dbcef2f61bb9ab3
Using Qt 5.10.1, built against Qt 5.10.1
Using 'OpenSSL 1.0.2n  7 Dec 2017'

Operating system: Xubuntu 18.04

OS language: english

Client package (From ownCloud or distro) (Linux only): testpilotcloud-client

Installation path of client: /usr/bin/testpilotcloud

Logs

Backtrace: https://gist.github.com/lefherz/7daf1b2bb6f5fc8eeb15f1ef79a0429e
@lefherz
Copy link
Contributor Author

lefherz commented May 30, 2018

Configuration of SFTP:

screenshot_2018-05-30_18-47-08

@jnweiger
Copy link
Contributor

jnweiger commented May 30, 2018 •
edited
Loading

Does not crash when running under gdb. Heisenbug...
Unconfigured SFTP folder means:

  • enter a nonexistant hostname e.g. 'foobar' as ftp server name.

Cannot yet reproduce the coredump, I only get lockups.
Reproducable with both, demo.owncloud.org and demo.owncloud.com

@jnweiger
Copy link
Contributor

@hodyroff maybe related to your SFTP issue?

@SamuAlfageme
Copy link
Contributor

Can reproduce. The moment the client detects the mounted folder I see in the logs:

05-31 09:21:08:951 [ debug sync.discovery ]	[ OCC::DiscoveryMainThread::singleDirectoryJobFirstDirectoryPermissionsSlot ]:	Permissions for root dir: "DNVCKR"
05-31 09:21:08:952 [ warning sync.discovery ]:	Missing properties: "SFTP" 2 0 1527751269 "M" "" "-0000001oc0v33t8034l"
05-31 09:21:08:952 [ debug sync.discovery ]	[ OCC::DiscoveryMainThread::singleDirectoryJobFinishedWithErrorSlot ]:	10011 "The server file discovery reply is missing data."
05-31 09:21:08:952 [ debug sync.networkjob ]	[ OCC::AbstractNetworkJob::slotFinished ]:	Network job OCC::LsColJob finished for ""
05-31 09:21:08:952 [ debug sync.discovery ]	[ OCC::DiscoveryJob::remote_vio_opendir_hook ]:	OCC::DiscoveryJob(0x7ff77b482a30)  ...Returned from main thread
05-31 09:21:08:953 [ debug sync.discovery ]	[ OCC::DiscoveryJob::remote_vio_opendir_hook ]:	10011 when opening  msg= "The server file discovery reply is missing data."
05-31 09:21:08:953 [ warning sync.csync.updater ]:	opendir failed for  - errno 10011
05-31 09:21:08:954 [ warning sync.engine ]:	ERROR during  csync_update :  "A HTTP transmission error happened. The server file discovery reply is missing data."
05-31 09:21:08:958 [ info sync.database ]:	Closing DB "/Users/salfageme/ownCloud3/._sync_b7fedb66744c.db"
05-31 09:21:08:958 [ debug sync.database ]	[ OCC::SyncJournalDb::commitTransaction ]:	No database Transaction to commit
05-31 09:21:08:959 [ info sync.engine ]:	CSync run took  179 ms
05-31 09:21:08:960 [ debug sync.localdiscoverytracker ]	[ OCC::LocalDiscoveryTracker::slotSyncFinished ]:	sync failed, keeping last sync's local discovery path list
05-31 09:21:08:981 [ info gui.folder ]:	Client version 2.5.0-daily20180528 (build 9870)  Qt 5.10.1  SSL  OpenSSL 1.0.2n  7 Dec 2017
05-31 09:21:08:981 [ warning gui.folder ]:	SyncEngine finished with ERROR

When looking at the discovery code that triggers this error:

client/src/libsync/discoveryphase.cpp

Lines 384 to 395 in bbf1843

if (file_stat->type == ItemTypeSkip
|| file_stat->size == -1
|| file_stat->modtime == -1
|| file_stat->remotePerm.isNull()
|| file_stat->etag.isEmpty()
|| file_stat->file_id.isEmpty()) {
_error = tr("The server file discovery reply is missing data.");
qCWarning(lcDiscovery)
<< "Missing properties:" << file << file_stat->type << file_stat->size
<< file_stat->modtime << file_stat->remotePerm.toString()
<< file_stat->etag << file_stat->file_id;
}

One can tell the server is including the SFTP directory on the results of the PROPFIND but did not set an ETAG for the mounted dir:

  <d:response>
    <d:href>/remote.php/dav/files/admin/SFTP/</d:href>
    <d:propstat>
      <d:prop>
        <d:resourcetype>
          <d:collection/>
        </d:resourcetype>
        <d:getetag>&quot;&quot;</d:getetag>
      </d:prop>
      <d:status>HTTP/1.1 200 OK</d:status>
    </d:propstat>
  </d:response>

Therefore the SFTP entry should never go into the selective sync list - but when setting up the local sync folder from scratch (either via new account or new FSC wizard) there it goes. Then clicking in the arrow means running a PROPFIND to https://<server>/remote.php/dav/files/admin/SFTP/ that returns an unhandled 503.

I just sent crash report bp-daa0c72c-ab49-420b-be99-0068b2180531 with the dump file from macOS.

¿Same condition was already triggered on #285?

@ogoffart ogoffart self-assigned this May 31, 2018
@ogoffart
Copy link
Contributor

I can reproduce.

Backtrace:

=4553== Thread 1:
==4553== Invalid read of size 8
==4553==    at 0x9C4FD10: QNetworkReply::error() const (in /usr/lib/libQt5Network.so.5.11.0)
==4553==    by 0x2C35FC: OCC::FolderStatusModel::slotLscolFinishedWithError(QNetworkReply*) (folderstatusmodel.cpp:746)
==4553==    by 0x2CF6F5: QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<QNetworkReply*>, void, void (OCC::FolderStatusModel::*)(QNetworkReply*)>::call(void (OCC::FolderStatusModel::*)(QNetworkReply*), OCC::FolderStatusModel*, void**) (qobjectdefs_impl.h:134)
==4553==    by 0x2CF652: void QtPrivate::FunctionPointer<void (OCC::FolderStatusModel::*)(QNetworkReply*)>::call<QtPrivate::List<QNetworkReply*>, void>(void (OCC::FolderStatusModel::*)(QNetworkReply*), OCC::FolderStatusModel*, void**) (qobjectdefs_impl.h:167)
==4553==    by 0x2CF575: QtPrivate::QSlotObject<void (OCC::FolderStatusModel::*)(QNetworkReply*), QtPrivate::List<QNetworkReply*>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (qobjectdefs_impl.h:396)
==4553==    by 0xA9BEB5F: QMetaObject::activate(QObject*, int, int, void**) (in /usr/lib/libQt5Core.so.5.11.0)
==4553==    by 0x54BAD75: OCC::LsColJob::finishedWithError(QNetworkReply*) (moc_networkjobs.cpp:509)
==4553==    by 0x541C341: OCC::LsColJob::finished() (networkjobs.cpp:381)
==4553==    by 0x541306E: OCC::AbstractNetworkJob::slotFinished() (abstractnetworkjob.cpp:234)
==4553==    by 0x5416B3A: QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, void (OCC::AbstractNetworkJob::*)()>::call(void (OCC::AbstractNetworkJob::*)(), OCC::AbstractNetworkJob*, void**) (qobjectdefs_impl.h:134)
==4553==    by 0x5416AA2: void QtPrivate::FunctionPointer<void (OCC::AbstractNetworkJob::*)()>::call<QtPrivate::List<>, void>(void (OCC::AbstractNetworkJob::*)(), OCC::AbstractNetworkJob*, void**) (qobjectdefs_impl.h:167)
==4553==    by 0x54169C5: QtPrivate::QSlotObject<void (OCC::AbstractNetworkJob::*)(), QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (qobjectdefs_impl.h:396)
==4553==  Address 0x23551078 is 8 bytes inside a block of size 16 free'd
==4553==    at 0x4C2E60B: operator delete(void*) (vg_replace_malloc.c:576)
==4553==    by 0x5411518: OCC::AbstractNetworkJob::setReply(QNetworkReply*) (abstractnetworkjob.cpp:78)
==4553==    by 0x541475A: OCC::AbstractNetworkJob::~AbstractNetworkJob() (abstractnetworkjob.cpp:287)
==4553==    by 0x54C41A2: OCC::LsColJob::~LsColJob() (networkjobs.h:65)
==4553==    by 0x54C41C8: OCC::LsColJob::~LsColJob() (networkjobs.h:65)
==4553==    by 0x2C1A3A: OCC::FolderStatusModel::SubFolderInfo::resetSubs(OCC::FolderStatusModel*, QModelIndex) (folderstatusmodel.cpp:1229)
==4553==    by 0x2C35F0: OCC::FolderStatusModel::slotLscolFinishedWithError(QNetworkReply*) (folderstatusmodel.cpp:744)
==4553==    by 0x2CF6F5: QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<QNetworkReply*>, void, void (OCC::FolderStatusModel::*)(QNetworkReply*)>::call(void (OCC::FolderStatusModel::*)(QNetworkReply*), OCC::FolderStatusModel*, void**) (qobjectdefs_impl.h:134)
==4553==    by 0x2CF652: void QtPrivate::FunctionPointer<void (OCC::FolderStatusModel::*)(QNetworkReply*)>::call<QtPrivate::List<QNetworkReply*>, void>(void (OCC::FolderStatusModel::*)(QNetworkReply*), OCC::FolderStatusModel*, void**) (qobjectdefs_impl.h:167)
==4553==    by 0x2CF575: QtPrivate::QSlotObject<void (OCC::FolderStatusModel::*)(QNetworkReply*), QtPrivate::List<QNetworkReply*>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (qobjectdefs_impl.h:396)
==4553==    by 0xA9BEB5F: QMetaObject::activate(QObject*, int, int, void**) (in /usr/lib/libQt5Core.so.5.11.0)
==4553==    by 0x54BAD75: OCC::LsColJob::finishedWithError(QNetworkReply*) (moc_networkjobs.cpp:509)
==4553==  Block was alloc'd at
==4553==    at 0x4C2D54F: operator new(unsigned long) (vg_replace_malloc.c:334)
==4553==    by 0x9C3C80D: QNetworkAccessManager::createRequest(QNetworkAccessManager::Operation, QNetworkRequest const&, QIODevice*) (in /usr/lib/libQt5Network.so.5.11.0)
==4553==    by 0x54059E9: OCC::AccessManager::createRequest(QNetworkAccessManager::Operation, QNetworkRequest const&, QIODevice*) (accessmanager.cpp:103)
==4553==    by 0x54B5065: OCC::HttpCredentialsAccessManager::createRequest(QNetworkAccessManager::Operation, QNetworkRequest const&, QIODevice*) (httpcredentials.cpp:88)
==4553==    by 0x9C39516: QNetworkAccessManager::sendCustomRequest(QNetworkRequest const&, QByteArray const&, QIODevice*) (in /usr/lib/libQt5Network.so.5.11.0)
==4553==    by 0x53D9C89: OCC::Account::sendRawRequest(QByteArray const&, QUrl const&, QNetworkRequest, QIODevice*) (account.cpp:257)
==4553==    by 0x5413456: OCC::AbstractNetworkJob::sendRequest(QByteArray const&, QUrl const&, QNetworkRequest, QIODevice*) (abstractnetworkjob.cpp:128)
==4553==    by 0x541B935: OCC::LsColJob::start() (networkjobs.cpp:345)
==4553==    by 0x2C1796: OCC::FolderStatusModel::fetchMore(QModelIndex const&) (folderstatusmodel.cpp:572)
==4553==    by 0x6780222: QTreeViewPrivate::layout(int, bool, bool) (in /usr/lib/libQt5Widgets.so.5.11.0)
==4553==    by 0x67852D1: QTreeViewPrivate::expand(int, bool) (in /usr/lib/libQt5Widgets.so.5.11.0)
==4553==    by 0x678679A: QTreeViewPrivate::expandOrCollapseItemAtPos(QPoint const&) (in /usr/lib/libQt5Widgets.so.5.11.0)
=

@ogoffart ogoffart added this to the 2.5.0 milestone May 31, 2018
ogoffart added a commit that referenced this issue May 31, 2018
@ogoffart
FolderStatusModel: Fix crash when there is an error while expanding f…
0793761 
…olders

In FolderStatusModel::slotLscolFinishedWithError, the call to parentInfo->resetSubs
deleted the 'job' and the reply 'r' which we accessed later to get the error code.

Fix this problem twice by
 1) Get the error code before caling resetSubs
 2) in FolderStatusModel::SubFolderInfo::resetSubs, call deleteLater instead of delete

Regression introduced in commit d69936e

Issue #6562
@ogoffart ogoffart mentioned this issue May 31, 2018
Merged
@SamuAlfageme SamuAlfageme mentioned this issue May 31, 2018
Merged
ogoffart added a commit that referenced this issue Jun 4, 2018
@ogoffart
FolderStatusModel: Fix crash when there is an error while expanding f…
76bb76a 
…olders

In FolderStatusModel::slotLscolFinishedWithError, the call to parentInfo->resetSubs
deleted the 'job' and the reply 'r' which we accessed later to get the error code.

Fix this problem twice by
 1) Get the error code before caling resetSubs
 2) in FolderStatusModel::SubFolderInfo::resetSubs, call deleteLater instead of delete

Regression introduced in commit d69936e

Issue #6562
@ogoffart ogoffart added the ReadyToTest QA, please validate the fix/enhancement label Jun 4, 2018
@SamuAlfageme
Copy link
Contributor

Crash no longer reproduces - closing here as it's fixed. 👍

The leftover collateral of not syncing anything when this error pops up (see my concerns on #6563 (comment)) can be tracked in #5859 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ogoffart ogoffart

Labels
ReadyToTest QA, please validate the fix/enhancement sev1-critical
Projects
None yet
Milestone
2.5.0
Development

No branches or pull requests
4 participants
@ogoffart @jnweiger @SamuAlfageme @lefherz