strepsirrhini/libreoffice-online
3
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

wsd: exclude readonly sessions from modification indication

Browse source 
Read only sessions cannot modify the document, so
we shouldn't consider input from such a session
as potentially modifying the document. This
becomes important during an abrupt disconnection
where we look at isPossiblyModified() as an
indicator for data-loss.

New unit-test that reproduces the corner-case
with disconnection. With graceful unloading
a previous unit-test verified that it worked
as expected.

Change-Id: Id84cda4f4599c559018247c32ea1205e154e4984
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
This commit is contained in:
Ashod Nakashian 2023-08-29 23:08:50 -04:00 committed by Ashod Nakashian
parent 8533175bd7
commit e6798e9062
2 changed files with 25 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
test/UnitWOPIFailUpload.cpp
View file

@ -287,14 +287,16 @@ private:
STATE_ENUM(Phase, Load, WaitLoadStatus, WaitModifiedStatus, WaitDocClose, Done) _phase; STATE_ENUM(Phase, Load, WaitLoadStatus, WaitModifiedStatus, WaitDocClose, Done) _phase;
const Scenario _scenario; const Scenario _scenario;
const bool _disconnect;
std::chrono::steady_clock::time_point _eventTime; std::chrono::steady_clock::time_point _eventTime;
public: public:
UnitWOPIReadOnly(Scenario scenario) UnitWOPIReadOnly(Scenario scenario, bool disconnect)
: WopiTestServer("UnitWOPIReadOnly_" + toStringShort(scenario)) : WopiTestServer("UnitWOPIReadOnly_" + toStringShort(scenario) + (disconnect ? "_X" : ""))
, _phase(Phase::Load) , _phase(Phase::Load)
, _scenario(scenario) , _scenario(scenario)
, _disconnect(disconnect)
{ {
} }
@ -408,10 +410,19 @@ public:
{ {
LOG_TST("No modified status on read-only document after waiting for " LOG_TST("No modified status on read-only document after waiting for "
<< elapsed); << elapsed);
TRANSITION_STATE(_phase, Phase::WaitDocClose);
_eventTime = std::chrono::steady_clock::now(); _eventTime = std::chrono::steady_clock::now();
LOG_TST("Saving the document");
WSD_CMD("save dontTerminateEdit=0 dontSaveIfUnmodified=0"); if (_disconnect)
{
TRANSITION_STATE(_phase, Phase::Done);
deleteSocketAt(0);
}
else
{
TRANSITION_STATE(_phase, Phase::WaitDocClose);
LOG_TST("Saving the document");
WSD_CMD("save dontTerminateEdit=0 dontSaveIfUnmodified=0");
}
} }
} }
break; break;
@ -745,12 +756,14 @@ public:
UnitBase** unit_create_wsd_multi(void) UnitBase** unit_create_wsd_multi(void)
{ {
return new UnitBase* [7] return new UnitBase* [9]
{ {
new UnitWOPIExpiredToken(), new UnitWOPIFailUpload(), new UnitWOPIExpiredToken(), new UnitWOPIFailUpload(),
new UnitWOPIReadOnly(UnitWOPIReadOnly::Scenario::ViewWithComment), new UnitWOPIReadOnly(UnitWOPIReadOnly::Scenario::ViewWithComment, /*disconnect=*/false),
new UnitWOPIReadOnly(UnitWOPIReadOnly::Scenario::Edit), new UnitFailUploadModified(), new UnitWOPIReadOnly(UnitWOPIReadOnly::Scenario::Edit, /*disconnect=*/false),
new UnitFailUplaodClose(), nullptr new UnitWOPIReadOnly(UnitWOPIReadOnly::Scenario::ViewWithComment, /*disconnect=*/true),
new UnitWOPIReadOnly(UnitWOPIReadOnly::Scenario::Edit, /*disconnect=*/true),
new UnitFailUploadModified(), new UnitFailUplaodClose(), nullptr
}; };
} }

5
wsd/ClientSession.cpp
View file

@ -799,7 +799,8 @@ bool ClientSession::_handleInput(const char *buffer, int length)
} }
else else
{ {
docBroker->updateLastModifyingActivityTime(); if (!isReadOnly())
docBroker->updateLastModifyingActivityTime();
return forwardToChild(std::string(buffer, length), docBroker); return forwardToChild(std::string(buffer, length), docBroker);
} }
} }
@ -1061,7 +1062,7 @@ bool ClientSession::_handleInput(const char *buffer, int length)
if (tokens.equals(0, "key")) if (tokens.equals(0, "key"))
_keyEvents++; _keyEvents++;
if (COOLProtocol::tokenIndicatesDocumentModification(tokens)) if (!isReadOnly() && COOLProtocol::tokenIndicatesDocumentModification(tokens))
{ {
docBroker->updateLastModifyingActivityTime(); docBroker->updateLastModifyingActivityTime();
} }