ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

LOCRDT editeng,sw: yrs weak link peer cursors for editengine

Browse Source 
Replace unreliable integer cursors with WeakRef cursors which maintain
their position automatically.

This requires yrs v0.23.1 with the yweak_read() function.

Unfortunately this crashes currently, so hide it behind #if YRS_WEAK.

https://github.com/y-crdt/y-crdt/issues/536

Change-Id: I74ec1eaa25c7cb554efd38ee632df041ac92d5c2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/183311
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
This commit is contained in:
Michael Stahl
2025-05-13 10:00:20 +02:00
parent 28c2be7c1f
commit ee7ab3d463
6 changed files with 138 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
editeng/inc/editdoc.hxx
+3
View File
@@ -51,6 +51,8 @@ class ImpEditEngine;
class IYrsTransactionSupplier;
typedef struct TransactionInner YTransaction;
typedef struct YTextEvent YTextEvent;
typedef struct Branch Branch;
typedef struct YOutput YOutput;
#endif
@@ -134,6 +136,7 @@ public:
void YrsWriteEEState();
void YrsReadEEState(YTransaction *, ImpEditEngine & rIEE);
void YrsApplyEEDelta(YTransaction *, YTextEvent const* pEvent, ImpEditEngine & rIEE);
bool YrsWriteEECursor(YTransaction *, Branch const& rArray, YOutput const* pCurrent, EditSelection const& rSelection);
::std::optional<EditSelection> YrsReadEECursor(::std::pair<int64_t, int64_t> point,
::std::optional<::std::pair<int64_t, int64_t>> oMark);
void YrsSetStyle(sal_Int32 nPara, ::std::u16string_view rStyle);
editeng/source/editeng/editdoc.cxx
+88 -1
View File
@@ -2712,12 +2712,98 @@ void EditDoc::YrsReadEEState(YTransaction *const pTxn, ImpEditEngine & rIEE)
rIEE.RemoveParagraph(nodes); // remove pre-existing one from InitDoc()
}
bool EditDoc::YrsWriteEECursor(YTransaction *const pTxn, Branch const& rArray,
YOutput const*const pCurrent, EditSelection const& rSelection)
{
EditSelection sel{rSelection};
sel.Adjust(*this); // unfortunately min/max naming may be misleading...
YrsWrite const yw{GetYrsWrite(m_pYrsSupplier, m_CommentId)};
uint32_t start{0};
for (auto paras{GetPos(sel.Min().GetNode())}; paras != 0; --paras)
{
start += GetObject(paras-1)->Len() + 1;
}
uint32_t end{start};
start += sel.Min().GetIndex();
if (sel.HasRange())
{
auto const nStartNode{GetPos(sel.Min().GetNode())};
for (auto paras{GetPos(sel.Max().GetNode()) - nStartNode};
paras != 0; --paras)
{
end += GetObject(nStartNode + paras-1)->Len() + 1;
}
end += sel.Max().GetIndex();
}
else
{
end = start;
}
if (pCurrent != nullptr && pCurrent->tag == Y_ARRAY
&& yarray_len(pCurrent->value.y_type) == 2
// could do without the comment id? no, empty-text leaves pBranch null!
&& yarray_get(pCurrent->value.y_type, pTxn, 0)->tag == Y_JSON_STR
&& strcmp(yarray_get(pCurrent->value.y_type, pTxn, 0)->value.str, m_CommentId.getStr()) == 0)
{
::std::unique_ptr<YOutput, YOutputDeleter> const pWeak{yarray_get(pCurrent->value.y_type, pTxn, 1)};
yvalidate(pWeak->tag == Y_WEAK_LINK);
Branch * pBranch{nullptr};
uint32_t oldStart{SAL_MAX_UINT32};
uint32_t oldEnd{SAL_MAX_UINT32};
yweak_read(pWeak->value.y_type, pTxn, &pBranch, &oldStart, &oldEnd);
assert(oldStart != SAL_MAX_UINT32 && oldEnd != SAL_MAX_UINT32);
if (oldStart == start && oldEnd == end)
{
return false;
}
}
Weak const*const pWeak{ytext_quote(yw.pText, pTxn, start, end, Y_FALSE, Y_TRUE)};
::std::vector<YInput> positions;
positions.push_back(yinput_string(m_CommentId.getStr()));
positions.push_back(yinput_weak(pWeak));
YInput const input{yinput_yarray(positions.data(), 2)};
yarray_remove_range(&rArray, pTxn, 1, 1);
yarray_insert_range(&rArray, pTxn, 1, &input, 1);
// yweak_destroy(pWeak); // NO! UAF
return true;
}
::std::optional<EditSelection> EditDoc::YrsReadEECursor(
::std::pair<int64_t, int64_t> const i_point,
::std::optional<::std::pair<int64_t, int64_t>> const i_oMark)
{
// TODO should not use ints here?
::std::optional<EditPaM> oMark;
#if defined(YRS_WEAK)
auto start{i_point.first}; // returned from yweak_read(), need to convert
ContentNode * pNode{nullptr};
for (decltype(Count()) i = 0; i < Count(); ++i)
{
pNode = GetObject(i);
if (start <= o3tl::make_unsigned(pNode->Len()))
{
break;
}
start -= pNode->Len() + 1;
}
yvalidate(start <= o3tl::make_unsigned(pNode->Len()));
EditPaM point{pNode, static_cast<sal_Int32>(start)};
if (i_oMark)
{
auto end{i_oMark->first};
for (decltype(Count()) i = 0; i < Count(); ++i)
{
pNode = GetObject(i);
if (end <= o3tl::make_unsigned(pNode->Len()))
{
break;
}
end -= pNode->Len() + 1;
}
yvalidate(end <= o3tl::make_unsigned(pNode->Len()));
oMark.emplace(pNode, static_cast<sal_Int32>(end));
}
#else
if (i_oMark)
{
// yvalidate(i_oMark->first < Count());
@@ -2750,6 +2836,7 @@ void EditDoc::YrsReadEEState(YTransaction *const pTxn, ImpEditEngine & rIEE)
return {};
}
EditPaM point{&rNode, static_cast<sal_Int32>(i_point.second)};
#endif
return EditSelection{point, oMark ? *oMark : point};
}
editeng/source/editeng/editview.cxx
+5
View File
@@ -372,6 +372,11 @@ void EditView::YrsGetSelectionRectangles(
}
}
bool EditView::YrsWriteEECursor(YTransaction *const pTxn, Branch const& rArray, YOutput const*const pCurrent)
{
return getEditEngine().GetEditDoc().YrsWriteEECursor(pTxn, rArray, pCurrent, getImpl().GetEditSelection());
}
void EditView::YrsApplyEECursor(OString const& rPeerId, OUString const& rAuthor,
::std::pair<int64_t, int64_t> const point,
::std::optional<::std::pair<int64_t, int64_t>> const oMark)
include/editeng/editview.hxx
+2
View File
@@ -42,6 +42,7 @@ class IYrsTransactionSupplier;
typedef struct TransactionInner YTransaction;
typedef struct YTextEvent YTextEvent;
typedef struct Branch Branch;
typedef struct YOutput YOutput;
#endif
class EditTextObject;
@@ -417,6 +418,7 @@ public:
void YrsReadEEState(YTransaction *);
void YrsApplyEEDelta(YTransaction *, YTextEvent const* pEvent);
OString GetYrsCommentId() const;
bool YrsWriteEECursor(YTransaction *, Branch const& rArray, YOutput const* pCurrent);
void YrsApplyEECursor(OString const& rPeerId, OUString const& rAuthor,
::std::pair<int64_t, int64_t> point,
::std::optional<::std::pair<int64_t, int64_t>> oMark);
include/editeng/yrs.hxx
+1
View File
@@ -9,6 +9,7 @@
#pragma once
#define YRS_WEAK
extern "C" {
#if defined(__clang__)
#pragma clang diagnostic push
sw/source/core/doc/DocumentStateManager.cxx
+39
View File
@@ -740,7 +740,12 @@ void YrsReadCursor(ObserveCursorState & rState, OString const& rPeerId,
{
Branch const*const pArray{rCursor.value.y_type};
auto const len{yarray_len(pArray)};
#if defined(YRS_WEAK)
if (len == 2
&& yarray_get(pArray, rState.pTxn, 0)->tag == Y_JSON_STR)
#else
if (len == 3 || len == 5)
#endif
{
::std::unique_ptr<YOutput, YOutputDeleter> const pComment{yarray_get(pArray, rState.pTxn, 0)};
yvalidate(pComment->tag == Y_JSON_STR && pComment->len < SAL_MAX_INT32);
@@ -748,6 +753,20 @@ void YrsReadCursor(ObserveCursorState & rState, OString const& rPeerId,
YrsInvalidateEECursors(rState, rPeerId, &commentId);
YrsInvalidateSwCursors(rState, rPeerId, rAuthor, false);
::std::optional<::std::pair<int64_t, int64_t>> oMark;
#if defined(YRS_WEAK)
::std::unique_ptr<YOutput, YOutputDeleter> const pWeak{yarray_get(pArray, rState.pTxn, 1)};
yvalidate(pWeak->tag == Y_WEAK_LINK);
Branch * pBranch{nullptr};
uint32_t start{SAL_MAX_UINT32};
uint32_t end{SAL_MAX_UINT32};
yweak_read(pWeak->value.y_type, rState.pTxn, &pBranch, &start, &end);
yvalidate(start < SAL_MAX_INT32 && end < SAL_MAX_INT32);
if (start != end)
{
oMark.emplace(end, -1);
}
::std::pair<int64_t, int64_t> const pos{start, -1};
#else
if (len == 5)
{
::std::unique_ptr<YOutput, YOutputDeleter> const pNode{
@@ -763,6 +782,7 @@ void YrsReadCursor(ObserveCursorState & rState, OString const& rPeerId,
::std::unique_ptr<YOutput, YOutputDeleter> const pContent{yarray_get(pArray, rState.pTxn, 2)};
yvalidate(pContent->tag == Y_JSON_INT);
::std::pair<int64_t, int64_t> const pos{pNode->value.integer, pContent->value.integer};
#endif
rState.CursorUpdates.emplace_back(rPeerId, ::std::optional<OString>{commentId}, ::std::optional<OUString>{rAuthor}, pos, oMark);
}
else if (len == 2 || len == 4)
@@ -901,6 +921,15 @@ extern "C" void observe_cursors(void *const pState, uint32_t count, YEvent const
YrsReadCursor(rState, peerId, pChange[2].values[0], author, false);
break;
}
#if defined(YRS_WEAK)
case Y_WEAK_LINK:
{
// not sure what this is, but yffi doesn't have any API
// for it, let's hope we can just ignore it
// YWeakLinkEvent const*const pEvent{&events[i].content.weak};
break;
}
#endif
default:
assert(false);
}
@@ -1263,6 +1292,12 @@ void DocumentStateManager::YrsNotifyCursorUpdate()
pShell->GetView().GetPostItMgr()->GetActiveSidebarWin()})
{
// TODO StickyIndex cannot be inserted into YDoc ?
#if defined(YRS_WEAK)
if (pWin->GetOutlinerView()->GetEditView().YrsWriteEECursor(pTxn, *pEntry->value.y_type, pCurrent.get()))
{
YrsCommitModified();
}
#else
ESelection const sel{pWin->GetOutlinerView()->GetSelection()};
::std::vector<YInput> positions;
// the ID of the comment
@@ -1301,6 +1336,7 @@ void DocumentStateManager::YrsNotifyCursorUpdate()
YrsCommitModified();
}
}
#endif
}
else if (!pShell->IsStdMode()
|| pShell->GetSelectedObjCount() != 0
@@ -1342,6 +1378,9 @@ void DocumentStateManager::YrsNotifyCursorUpdate()
{
if (pCurrent == nullptr || pCurrent->tag != Y_ARRAY
|| yarray_len(pCurrent->value.y_type) != 2
#if defined(YRS_WEAK)
|| yarray_get(pCurrent->value.y_type, pTxn, 0)->tag != Y_JSON_INT
#endif
|| yarray_get(pCurrent->value.y_type, pTxn, 0)->value.integer != positions[0].value.integer
|| yarray_get(pCurrent->value.y_type, pTxn, 1)->value.integer != positions[1].value.integer)
{