From 988a23963797ca1d38910ac6b3b04cb8887274a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Mon, 24 Oct 2016 10:04:33 +0200 Subject: [PATCH] [server] Ensure we have a DataSource on the DataDevice in setSelection Summary: SeatInterface provides a way to set the current selection. This method did not verify whether the new DataDeviceInterface actually has a DataSourceInterface. If there is no DataSourceInterface on that DataDeviceInterface the selection should not be sent to the current selection owner. This results in a crash as DataOfferInterface (correctly) doesn't expect the passed in DataSourceInterface to be null. To ensure we don't hit this again the DataOfferInterface ctor gained an Q_ASSERT to validate the DataSourceInterface. Reviewers: #plasma_on_wayland Subscribers: plasma-devel Tags: #plasma_on_wayland Differential Revision: https://phabricator.kde.org/D3148 --- .../autotests/client/test_wayland_seat.cpp | 46 +++++++++++++++++++ src/wayland/dataoffer_interface.cpp | 1 + src/wayland/seat_interface.cpp | 2 +- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/wayland/autotests/client/test_wayland_seat.cpp b/src/wayland/autotests/client/test_wayland_seat.cpp index 0cbab56d08..533aae058f 100644 --- a/src/wayland/autotests/client/test_wayland_seat.cpp +++ b/src/wayland/autotests/client/test_wayland_seat.cpp @@ -79,6 +79,7 @@ private Q_SLOTS: void testCast(); void testDestroy(); void testSelection(); + void testSelectionNoDataSource(); void testTouch(); void testDisconnect(); void testPointerEnterOnUnboundSurface(); @@ -1552,6 +1553,51 @@ void TestWaylandSeat::testSelection() QVERIFY(cancelledSpy.wait()); } +void TestWaylandSeat::testSelectionNoDataSource() +{ + // this test verifies that the server doesn't crash when using setSelection with + // a DataDevice which doesn't have a DataSource yet + using namespace KWayland::Client; + using namespace KWayland::Server; + // first create the DataDevice + QScopedPointer ddmi(m_display->createDataDeviceManager()); + ddmi->create(); + QSignalSpy ddiCreatedSpy(ddmi.data(), &DataDeviceManagerInterface::dataDeviceCreated); + QVERIFY(ddiCreatedSpy.isValid()); + Registry registry; + QSignalSpy dataDeviceManagerSpy(®istry, &Registry::dataDeviceManagerAnnounced); + QVERIFY(dataDeviceManagerSpy.isValid()); + registry.setEventQueue(m_queue); + registry.create(m_connection->display()); + QVERIFY(registry.isValid()); + registry.setup(); + + QVERIFY(dataDeviceManagerSpy.wait()); + QScopedPointer ddm(registry.createDataDeviceManager(dataDeviceManagerSpy.first().first().value(), + dataDeviceManagerSpy.first().last().value())); + QVERIFY(ddm->isValid()); + + QScopedPointer dd(ddm->getDataDevice(m_seat)); + QVERIFY(dd->isValid()); + QVERIFY(ddiCreatedSpy.wait()); + auto ddi = ddiCreatedSpy.first().first().value(); + QVERIFY(ddi); + + // now create a surface and pass it keyboard focus + QSignalSpy surfaceCreatedSpy(m_compositorInterface, &CompositorInterface::surfaceCreated); + QVERIFY(surfaceCreatedSpy.isValid()); + QScopedPointer surface(m_compositor->createSurface()); + QVERIFY(surface->isValid()); + QVERIFY(surfaceCreatedSpy.wait()); + auto serverSurface = surfaceCreatedSpy.first().first().value(); + QVERIFY(!m_seatInterface->selection()); + m_seatInterface->setFocusedKeyboardSurface(serverSurface); + QCOMPARE(m_seatInterface->focusedKeyboardSurface(), serverSurface); + + // now let's set the selection + m_seatInterface->setSelection(ddi); +} + void TestWaylandSeat::testTouch() { using namespace KWayland::Client; diff --git a/src/wayland/dataoffer_interface.cpp b/src/wayland/dataoffer_interface.cpp index 936f8359a7..0b04653f7f 100644 --- a/src/wayland/dataoffer_interface.cpp +++ b/src/wayland/dataoffer_interface.cpp @@ -93,6 +93,7 @@ void DataOfferInterface::Private::receive(const QString &mimeType, qint32 fd) DataOfferInterface::DataOfferInterface(DataSourceInterface *source, DataDeviceInterface *parentInterface, wl_resource *parentResource) : Resource(new Private(source, parentInterface, this, parentResource)) { + Q_ASSERT(source); connect(source, &DataSourceInterface::mimeTypeOffered, this, [this](const QString &mimeType) { Q_D(); diff --git a/src/wayland/seat_interface.cpp b/src/wayland/seat_interface.cpp index 452c2c7b2c..28c8618d26 100644 --- a/src/wayland/seat_interface.cpp +++ b/src/wayland/seat_interface.cpp @@ -1344,7 +1344,7 @@ void SeatInterface::setSelection(DataDeviceInterface *dataDevice) d->cancelPreviousSelection(dataDevice); d->currentSelection = dataDevice; if (d->keys.focus.selection) { - if (dataDevice) { + if (dataDevice && dataDevice->selection()) { d->keys.focus.selection->sendSelection(dataDevice); } else { d->keys.focus.selection->sendClearSelection();