From 243d94875dc9c24932c2aec5ec44cf31e310d006 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Caol=C3=A1n=20McNamara?= <caolanm@redhat.com>

Date: Thu, 21 Dec 2017 16:19:42 +0000

Subject: [PATCH] rhbz#1527945 segv on failed open of password protected doc in

 gnome-documents

Change-Id: I1bec502ae20d03214c673d8911792c89669c0fe9

Reviewed-on: https://gerrit.libreoffice.org/46923

Tested-by: Jenkins <ci@libreoffice.org>

Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>

(cherry picked from commit a61f45499856aad9910d82af1312a163504c15c2)

Lok: number callback enum for easier debugging

Since the entries and their order are part of the

public API, and will not change, numbering them

makes it easier to trap particular callbacks by

their number (as that's what shows in logs and

the debugger).

Change-Id: Ife2fe3e601ce3dce0939363d748fcb54d3c85fd4

Reviewed-on: https://gerrit.libreoffice.org/31257

Tested-by: Jenkins <ci@libreoffice.org>

Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>

(cherry picked from commit 719f7cb94ce783349fb1cf366a78edd9996d3e37)

lokdocview: use std::string where possible

This also prevents an invalid memory access: we were storing the pointer

to a temporary in the struct and then using it after temporary was gone.

Change-Id: I2b6d9df16bc24b222095ccbf45c9f0bd9c86ed65

(cherry picked from commit 9d2e0d60c0381a4bb23fada14c80e032b68bf2a8)

Reviewed-on: https://gerrit.libreoffice.org/46951

Tested-by: Jenkins <ci@libreoffice.org>

Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>

(cherry picked from commit 4ed5f5b28aaaf4522b71f32fb2f9f6a960dbff69)

---

 include/LibreOfficeKit/LibreOfficeKitEnums.h | 64 ++++++++++++++--------------

 libreofficekit/source/gtk/lokdocview.cxx     | 44 +++++++++----------

 2 files changed, 53 insertions(+), 55 deletions(-)

diff --git a/include/LibreOfficeKit/LibreOfficeKitEnums.h b/include/LibreOfficeKit/LibreOfficeKitEnums.h

index 187fa9812e60..d2ccc0fdccd8 100644

--- a/include/LibreOfficeKit/LibreOfficeKitEnums.h

+++ b/include/LibreOfficeKit/LibreOfficeKitEnums.h

@@ -93,13 +93,13 @@ typedef enum

      *

      * @see LOK_FEATURE_PART_IN_INVALIDATION_CALLBACK.

      */

-    LOK_CALLBACK_INVALIDATE_TILES,

+    LOK_CALLBACK_INVALIDATE_TILES = 0,

     /**

      * The size and/or the position of the visible cursor changed.

      *

      * Rectangle format is the same as LOK_CALLBACK_INVALIDATE_TILES.

      */

-    LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR,

+    LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR = 1,

     /**

      * The list of rectangles representing the current text selection changed.

      *

@@ -108,7 +108,7 @@ typedef enum

      * LOK_CALLBACK_INVALIDATE_TILES. When there is no selection, an empty

      * string is provided.

      */

-    LOK_CALLBACK_TEXT_SELECTION,

+    LOK_CALLBACK_TEXT_SELECTION = 2,

     /**

      * The position and size of the cursor rectangle at the text

      * selection start. It is used to draw the selection handles.

@@ -118,7 +118,7 @@ typedef enum

      *

      * Rectangle format is the same as LOK_CALLBACK_INVALIDATE_TILES.

      */

-    LOK_CALLBACK_TEXT_SELECTION_START,

+    LOK_CALLBACK_TEXT_SELECTION_START = 3,

     /**

      * The position and size of the cursor rectangle at the text

      * selection end. It is used to draw the selection handles.

@@ -128,7 +128,7 @@ typedef enum

      *

      * Rectangle format is the same as LOK_CALLBACK_INVALIDATE_TILES.

      */

-    LOK_CALLBACK_TEXT_SELECTION_END,

+    LOK_CALLBACK_TEXT_SELECTION_END = 4,

     /**

      * The blinking text cursor is now visible or not.

      *

@@ -137,26 +137,26 @@ typedef enum

      * LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR once it becomes false. Payload is

      * either the "true" or the "false" string.

      */

-    LOK_CALLBACK_CURSOR_VISIBLE,

+    LOK_CALLBACK_CURSOR_VISIBLE = 5,

     /**

      * The size and/or the position of the graphic selection changed.

      *

      * Rectangle format is the same as LOK_CALLBACK_INVALIDATE_TILES.

      */

-    LOK_CALLBACK_GRAPHIC_SELECTION,

+    LOK_CALLBACK_GRAPHIC_SELECTION = 6,

     /**

      * User clicked on an hyperlink that should be handled by other

      * applications accordingly.

      */

-    LOK_CALLBACK_HYPERLINK_CLICKED,

+    LOK_CALLBACK_HYPERLINK_CLICKED = 7,

     /**

      * Emit state update to the client.

      * For example, when cursor is on bold text, this callback is triggered

      * with payload: ".uno:Bold=true"

      */

-    LOK_CALLBACK_STATE_CHANGED,

+    LOK_CALLBACK_STATE_CHANGED = 8,

     /**

      * Start a "status indicator" (here restricted to a progress bar type

@@ -172,25 +172,25 @@ typedef enum

      * loading a document and then constructing a LibreOfficeKitDocument

      * object.

      */

-    LOK_CALLBACK_STATUS_INDICATOR_START,

+    LOK_CALLBACK_STATUS_INDICATOR_START = 9,

     /**

      * Sets the numeric value of the status indicator.

      * The payload should be a percentage, an integer between 0 and 100.

      */

-    LOK_CALLBACK_STATUS_INDICATOR_SET_VALUE,

+    LOK_CALLBACK_STATUS_INDICATOR_SET_VALUE = 10,

     /**

      * Ends the status indicator.

      *

      * Not necessarily ever emitted.

      */

-    LOK_CALLBACK_STATUS_INDICATOR_FINISH,

+    LOK_CALLBACK_STATUS_INDICATOR_FINISH = 11,

     /**

      * No match was found for the search input

      */

-    LOK_CALLBACK_SEARCH_NOT_FOUND,

+    LOK_CALLBACK_SEARCH_NOT_FOUND = 12,

     /**

      * Size of the document changed.

@@ -198,14 +198,14 @@ typedef enum

      * Payload format is "width, height", i.e. clients get the new size without

      * having to do an explicit lok::Document::getDocumentSize() call.

      */

-    LOK_CALLBACK_DOCUMENT_SIZE_CHANGED,

+    LOK_CALLBACK_DOCUMENT_SIZE_CHANGED = 13,

     /**

      * The current part number is changed.

      *

      * Payload is a single 0-based integer.

      */

-    LOK_CALLBACK_SET_PART,

+    LOK_CALLBACK_SET_PART = 14,

     /**

      * Selection rectangles of the search result when find all is performed.

@@ -231,7 +231,7 @@ typedef enum

      * - searchResultSelection is an array of part-number and rectangle list

      *   pairs, in LOK_CALLBACK_SET_PART / LOK_CALLBACK_TEXT_SELECTION format.

      */

-    LOK_CALLBACK_SEARCH_RESULT_SELECTION,

+    LOK_CALLBACK_SEARCH_RESULT_SELECTION = 15,

     /**

      * Result of the UNO command execution when bNotifyWhenFinished was set

@@ -246,26 +246,26 @@ typedef enum

      *     // TODO "result": "..."  // UNO Any converted to JSON (not implemented yet)

      * }

      */

-    LOK_CALLBACK_UNO_COMMAND_RESULT,

+    LOK_CALLBACK_UNO_COMMAND_RESULT = 16,

     /**

      * The size and/or the position of the cell cursor changed.

      *

      * Rectangle format is the same as LOK_CALLBACK_INVALIDATE_TILES.

      */

-    LOK_CALLBACK_CELL_CURSOR,

+    LOK_CALLBACK_CELL_CURSOR = 17,

     /**

      * The current mouse pointer style.

      *

      * Payload is a css mouse pointer style.

      */

-    LOK_CALLBACK_MOUSE_POINTER,

+    LOK_CALLBACK_MOUSE_POINTER = 18,

     /**

      * The text content of the formula bar in Calc.

      */

-    LOK_CALLBACK_CELL_FORMULA,

+    LOK_CALLBACK_CELL_FORMULA = 19,

     /**

      * Loading a document requires a password.

@@ -274,7 +274,7 @@ typedef enum

      * lok::Office::setDocumentPassword().  The document cannot be loaded

      * without the password.

      */

-    LOK_CALLBACK_DOCUMENT_PASSWORD,

+    LOK_CALLBACK_DOCUMENT_PASSWORD = 20,

     /**

      * Editing a document requires a password.

@@ -282,7 +282,7 @@ typedef enum

      * Loading the document is blocked until the password is provided via

      * lok::Office::setDocumentPassword().

      */

-    LOK_CALLBACK_DOCUMENT_PASSWORD_TO_MODIFY,

+    LOK_CALLBACK_DOCUMENT_PASSWORD_TO_MODIFY = 21,

     /**

      * An error happened.

@@ -296,7 +296,7 @@ typedef enum

      *     "message": freeform description

      * }

      */

-    LOK_CALLBACK_ERROR,

+    LOK_CALLBACK_ERROR = 22,

     /**

      * Context menu structure

@@ -318,7 +318,7 @@ typedef enum

      *

      *     {"text": "label text3", "type": "command", "command": ".uno:Something3", "checktype": "checkmark|radio|auto", "checked": "true|false"}

      */

-    LOK_CALLBACK_CONTEXT_MENU,

+    LOK_CALLBACK_CONTEXT_MENU = 23,

     /**

      * The size and/or the position of the view cursor changed. A view cursor

@@ -334,7 +334,7 @@ typedef enum

      * - viewId is a value returned earlier by lok::Document::createView()

      * - rectangle uses the format of LOK_CALLBACK_INVALIDATE_VISIBLE_CURSOR

      */

-    LOK_CALLBACK_INVALIDATE_VIEW_CURSOR,

+    LOK_CALLBACK_INVALIDATE_VIEW_CURSOR = 24,

     /**

      * The text selection in one of the other views has changed.

@@ -349,7 +349,7 @@ typedef enum

      * - viewId is a value returned earlier by lok::Document::createView()

      * - selection uses the format of LOK_CALLBACK_TEXT_SELECTION.

      */

-    LOK_CALLBACK_TEXT_VIEW_SELECTION,

+    LOK_CALLBACK_TEXT_VIEW_SELECTION = 25,

     /**

      * The cell cursor in one of the other views has changed.

@@ -364,7 +364,7 @@ typedef enum

      * - viewId is a value returned earlier by lok::Document::createView()

      * - rectangle uses the format of LOK_CALLBACK_CELL_CURSOR.

      */

-    LOK_CALLBACK_CELL_VIEW_CURSOR,

+    LOK_CALLBACK_CELL_VIEW_CURSOR = 26,

     /**

      * The size and/or the position of a graphic selection in one of the other

@@ -380,7 +380,7 @@ typedef enum

      * - viewId is a value returned earlier by lok::Document::createView()

      * - selection uses the format of LOK_CALLBACK_INVALIDATE_TILES.

      */

-    LOK_CALLBACK_GRAPHIC_VIEW_SELECTION,

+    LOK_CALLBACK_GRAPHIC_VIEW_SELECTION = 27,

     /**

      * The blinking text cursor in one of the other views is now visible or

@@ -396,7 +396,7 @@ typedef enum

      * - viewId is a value returned earlier by lok::Document::createView()

      * - visible uses the format of LOK_CALLBACK_CURSOR_VISIBLE.

      */

-    LOK_CALLBACK_VIEW_CURSOR_VISIBLE,

+    LOK_CALLBACK_VIEW_CURSOR_VISIBLE = 28,

     /**

      * The size and/or the position of a lock rectangle in one of the other

@@ -412,7 +412,7 @@ typedef enum

      * - viewId is a value returned earlier by lok::Document::createView()

      * - rectangle uses the format of LOK_CALLBACK_INVALIDATE_TILES.

      */

-    LOK_CALLBACK_VIEW_LOCK,

+    LOK_CALLBACK_VIEW_LOCK = 29,

     /**

      * The size of the change tracking table has changed.

@@ -437,7 +437,7 @@ typedef enum

      * - 'action' is either 'Add' or 'Remove', depending on if this is an

      *   insertion into the table or a removal.

      */

-    LOK_CALLBACK_REDLINE_TABLE_SIZE_CHANGED,

+    LOK_CALLBACK_REDLINE_TABLE_SIZE_CHANGED = 30,

     /**

      * An entry in the change tracking table has been modified.

@@ -461,7 +461,7 @@ typedef enum

      *

      * - 'action' is 'Modify'.

      */

-    LOK_CALLBACK_REDLINE_TABLE_ENTRY_MODIFIED,

+    LOK_CALLBACK_REDLINE_TABLE_ENTRY_MODIFIED = 31,

 }

 LibreOfficeKitCallbackType;

diff --git a/libreofficekit/source/gtk/lokdocview.cxx b/libreofficekit/source/gtk/lokdocview.cxx

index 0b48cf8b7373..bbf64bf9ec9a 100644

--- a/libreofficekit/source/gtk/lokdocview.cxx

+++ b/libreofficekit/source/gtk/lokdocview.cxx

@@ -77,9 +77,9 @@ struct ViewRectangles

 /// Private struct used by this GObject type

 struct LOKDocViewPrivateImpl

 {

-    const gchar* m_aLOPath;

-    const gchar* m_pUserProfileURL;

-    const gchar* m_aDocPath;

+    std::string m_aLOPath;

+    std::string m_aUserProfileURL;

+    std::string m_aDocPath;

     std::string m_aRenderingArguments;

     gdouble m_nLoadProgress;

     gboolean m_bIsLoading;

@@ -195,10 +195,7 @@ struct LOKDocViewPrivateImpl

     std::map<int, ViewRectangle> m_aViewLockRectangles;

     LOKDocViewPrivateImpl()

-        : m_aLOPath(nullptr),

-        m_pUserProfileURL(nullptr),

-        m_aDocPath(nullptr),

-        m_nLoadProgress(0),

+        : m_nLoadProgress(0),

         m_bIsLoading(false),

         m_bCanZoomIn(true),

         m_bCanZoomOut(true),

@@ -1736,7 +1733,7 @@ renderOverlay(LOKDocView* pDocView, cairo_t* pCairo)

     if (priv->m_bEdit && priv->m_bCursorVisible && !isEmptyRectangle(priv->m_aVisibleCursor) && priv->m_aTextSelectionRectangles.empty())

     {

         // Have a cursor, but no selection: we need the middle handle.

-        gchar* handleMiddlePath = g_strconcat (priv->m_aLOPath, CURSOR_HANDLE_DIR, "handle_image_middle.png", nullptr);

+        gchar* handleMiddlePath = g_strconcat (priv->m_aLOPath.c_str(), CURSOR_HANDLE_DIR, "handle_image_middle.png", nullptr);

         if (!priv->m_pHandleMiddle)

         {

             priv->m_pHandleMiddle = cairo_image_surface_create_from_png(handleMiddlePath);

@@ -1764,7 +1761,7 @@ renderOverlay(LOKDocView* pDocView, cairo_t* pCairo)

         if (!isEmptyRectangle(priv->m_aTextSelectionStart))

         {

             // Have a start position: we need a start handle.

-            gchar* handleStartPath = g_strconcat (priv->m_aLOPath, CURSOR_HANDLE_DIR, "handle_image_start.png", nullptr);

+            gchar* handleStartPath = g_strconcat (priv->m_aLOPath.c_str(), CURSOR_HANDLE_DIR, "handle_image_start.png", nullptr);

             if (!priv->m_pHandleStart)

             {

                 priv->m_pHandleStart = cairo_image_surface_create_from_png(handleStartPath);

@@ -1776,7 +1773,7 @@ renderOverlay(LOKDocView* pDocView, cairo_t* pCairo)

         if (!isEmptyRectangle(priv->m_aTextSelectionEnd))

         {

             // Have a start position: we need an end handle.

-            gchar* handleEndPath = g_strconcat (priv->m_aLOPath, CURSOR_HANDLE_DIR, "handle_image_end.png", nullptr);

+            gchar* handleEndPath = g_strconcat (priv->m_aLOPath.c_str(), CURSOR_HANDLE_DIR, "handle_image_end.png", nullptr);

             if (!priv->m_pHandleEnd)

             {

                 priv->m_pHandleEnd = cairo_image_surface_create_from_png(handleEndPath);

@@ -2207,8 +2204,7 @@ openDocumentInThread (gpointer data)

     }

     priv->m_pOffice->pClass->registerCallback(priv->m_pOffice, globalCallbackWorker, pDocView);

-    priv->m_pDocument = priv->m_pOffice->pClass->documentLoad( priv->m_pOffice, priv->m_aDocPath );

-    priv->m_eDocumentType = static_cast<LibreOfficeKitDocumentType>(priv->m_pDocument->pClass->getDocumentType(priv->m_pDocument));

+    priv->m_pDocument = priv->m_pOffice->pClass->documentLoad( priv->m_pOffice, priv->m_aDocPath.c_str() );

     if ( !priv->m_pDocument )

     {

         char *pError = priv->m_pOffice->pClass->getError( priv->m_pOffice );

@@ -2216,6 +2212,7 @@ openDocumentInThread (gpointer data)

     }

     else

     {

+        priv->m_eDocumentType = static_cast<LibreOfficeKitDocumentType>(priv->m_pDocument->pClass->getDocumentType(priv->m_pDocument));

         gdk_threads_add_idle(postDocumentLoad, pDocView);

         g_task_return_boolean (task, true);

     }

@@ -2469,16 +2466,17 @@ static void lok_doc_view_set_property (GObject* object, guint propId, const GVal

     switch (propId)

     {

     case PROP_LO_PATH:

-        priv->m_aLOPath = g_value_dup_string (value);

+        priv->m_aLOPath = g_value_get_string (value);

         break;

     case PROP_LO_POINTER:

         priv->m_pOffice = static_cast<LibreOfficeKit*>(g_value_get_pointer(value));

         break;

     case PROP_USER_PROFILE_URL:

-        priv->m_pUserProfileURL = g_value_dup_string(value);

+        if (const gchar* pUserProfile = g_value_get_string(value))

+            priv->m_aUserProfileURL = pUserProfile;

         break;

     case PROP_DOC_PATH:

-        priv->m_aDocPath = g_value_dup_string (value);

+        priv->m_aDocPath = g_value_get_string (value);

         break;

     case PROP_DOC_POINTER:

         priv->m_pDocument = static_cast<LibreOfficeKitDocument*>(g_value_get_pointer(value));

@@ -2523,16 +2521,16 @@ static void lok_doc_view_get_property (GObject* object, guint propId, GValue *va

     switch (propId)

     {

     case PROP_LO_PATH:

-        g_value_set_string (value, priv->m_aLOPath);

+        g_value_set_string (value, priv->m_aLOPath.c_str());

         break;

     case PROP_LO_POINTER:

         g_value_set_pointer(value, priv->m_pOffice);

         break;

     case PROP_USER_PROFILE_URL:

-        g_value_set_string(value, priv->m_pUserProfileURL);

+        g_value_set_string(value, priv->m_aUserProfileURL.c_str());

         break;

     case PROP_DOC_PATH:

-        g_value_set_string (value, priv->m_aDocPath);

+        g_value_set_string (value, priv->m_aDocPath.c_str());

         break;

     case PROP_DOC_POINTER:

         g_value_set_pointer(value, priv->m_pDocument);

@@ -2632,14 +2630,14 @@ static gboolean lok_doc_view_initable_init (GInitable *initable, GCancellable* /

     if (priv->m_pOffice != nullptr)

         return TRUE;

-    priv->m_pOffice = lok_init_2(priv->m_aLOPath, priv->m_pUserProfileURL);

+    priv->m_pOffice = lok_init_2(priv->m_aLOPath.c_str(), priv->m_aUserProfileURL.empty() ? nullptr : priv->m_aUserProfileURL.c_str());

     if (priv->m_pOffice == nullptr)

     {

         g_set_error (error,

                      g_quark_from_static_string ("LOK initialization error"), 0,

                      "Failed to get LibreOfficeKit context. Make sure path (%s) is correct",

-                     priv->m_aLOPath);

+                     priv->m_aLOPath.c_str());

         return FALSE;

     }

     priv->m_nLOKFeatures |= LOK_FEATURE_PART_IN_INVALIDATION_CALLBACK;

@@ -3118,8 +3116,8 @@ SAL_DLLPUBLIC_EXPORT GtkWidget* lok_doc_view_new_from_widget(LOKDocView* pOldLOK

 {

     LOKDocViewPrivate& pOldPriv = getPrivate(pOldLOKDocView);

     GtkWidget* pNewDocView = GTK_WIDGET(g_initable_new(LOK_TYPE_DOC_VIEW, /*cancellable=*/nullptr, /*error=*/nullptr,

-                                                       "lopath", pOldPriv->m_aLOPath,

-                                                       "userprofileurl", pOldPriv->m_pUserProfileURL,

+                                                       "lopath", pOldPriv->m_aLOPath.c_str(),

+                                                       "userprofileurl", pOldPriv->m_aUserProfileURL.c_str(),

                                                        "lopointer", pOldPriv->m_pOffice,

                                                        "docpointer", pOldPriv->m_pDocument,

                                                        "halign", GTK_ALIGN_CENTER,

@@ -3165,7 +3163,7 @@ lok_doc_view_open_document (LOKDocView* pDocView,

     LOEvent* pLOEvent = new LOEvent(LOK_LOAD_DOC);

     pLOEvent->m_pPath = pPath;

-    priv->m_aDocPath = pPath;

+    g_object_set(G_OBJECT(pDocView), "docpath", pPath, nullptr);

     if (pRenderingArguments)

         priv->m_aRenderingArguments = pRenderingArguments;

     g_task_set_task_data(task, pLOEvent, LOEvent::destroy);

-- 

2.14.3