common/JsonUtil.hpp    |  146 ++++++++++++++++++++++++++++++++++++++++++
 common/Util.hpp        |    3 
 test/WhiteBoxTests.cpp |   35 ++++++++++
 wsd/Storage.cpp        |  169 ++++++++++---------------------------------------
 4 files changed, 218 insertions(+), 135 deletions(-)

New commits:
commit 5befd0803ae86747bc3c50aa5a40f6312bcf667b
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date:   Sat Feb 17 17:32:41 2018 -0500

    wsd: improved wopi info parsing
    
    Better logging during wopi info parsing,
    especially upon failures.
    Refactored the code from Storage.cpp into
    JsonUtil.hpp.
    Minor optimizations.
    
    Add unit-tests for the parsing logic.
    
    Change-Id: Ifebc3f6b7030a6c7b3b399786633f6b5e8737478
    Reviewed-on: https://gerrit.libreoffice.org/49927
    Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
    Tested-by: Ashod Nakashian <ashnak...@gmail.com>

diff --git a/common/JsonUtil.hpp b/common/JsonUtil.hpp
new file mode 100644
index 00000000..fe63a469
--- /dev/null
+++ b/common/JsonUtil.hpp
@@ -0,0 +1,146 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; 
fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#ifndef INCLUDED_JSONUTIL_HPP
+#define INCLUDED_JSONUTIL_HPP
+
+#include <cstddef>
+#include <set>
+#include <string>
+
+#include <Poco/JSON/Object.h>
+#include <Poco/JSON/Parser.h>
+
+#include <Log.hpp>
+#include <JsonUtil.hpp>
+
+namespace JsonUtil
+{
+
+// Parse the json string and fill the Poco::JSON object
+// Returns true if parsing successful otherwise false
+bool parseJSON(const std::string& json, Poco::JSON::Object::Ptr& object)
+{
+    bool success = false;
+    const size_t index = json.find_first_of('{');
+    if (index != std::string::npos)
+    {
+        const std::string stringJSON = json.substr(index);
+        Poco::JSON::Parser parser;
+        const Poco::Dynamic::Var result = parser.parse(stringJSON);
+        object = result.extract<Poco::JSON::Object::Ptr>();
+        success = true;
+    }
+
+    return success;
+}
+
+inline
+int getLevenshteinDist(const std::string& string1, const std::string& string2)
+{
+    int matrix[string1.size() + 1][string2.size() + 1];
+    std::memset(matrix, 0, sizeof(matrix[0][0]) * (string1.size() + 1) * 
(string2.size() + 1));
+
+    for (size_t i = 0; i < string1.size() + 1; i++)
+    {
+        for (size_t j = 0; j < string2.size() + 1; j++)
+        {
+            if (i == 0)
+            {
+                matrix[i][j] = j;
+            }
+            else if (j == 0)
+            {
+                matrix[i][j] = i;
+            }
+            else if (string1[i - 1] == string2[j - 1])
+            {
+                matrix[i][j] = matrix[i - 1][j - 1];
+            }
+            else
+            {
+                matrix[i][j] = 1 + std::min(std::min(matrix[i][j - 1], 
matrix[i - 1][j]),
+                                            matrix[i - 1][j - 1]);
+            }
+        }
+    }
+
+    return matrix[string1.size()][string2.size()];
+}
+
+// Gets value for `key` directly from the given JSON in `object`
+template <typename T>
+T getJSONValue(const Poco::JSON::Object::Ptr &object, const std::string& key)
+{
+    try
+    {
+        const Poco::Dynamic::Var valueVar = object->get(key);
+        return valueVar.convert<T>();
+    }
+    catch (const Poco::Exception& exc)
+    {
+        LOG_ERR("getJSONValue for [" << key << "]: " << exc.displayText() <<
+                (exc.nested() ? " (" + exc.nested()->displayText() + ")" : 
""));
+    }
+
+    return T();
+}
+
+/// Function that searches `object` for `key` and warns if there are minor 
mis-spellings involved.
+/// Upon successfull search, fills `value` with value found in object.
+/// Removes the entry from the JSON object if @bRemove == true.
+template <typename T>
+bool findJSONValue(Poco::JSON::Object::Ptr &object, const std::string& key, T& 
value, bool bRemove = true)
+{
+    std::string keyLower(key);
+    std::transform(begin(key), end(key), begin(keyLower), ::tolower);
+
+    std::vector<std::string> propertyNames;
+    object->getNames(propertyNames);
+
+    // Check each property name against given key
+    // and warn for mis-spells with tolerance of 2.
+    for (const std::string& userInput: propertyNames)
+    {
+        if (key != userInput)
+        {
+            std::string userInputLower(userInput);
+            std::transform(begin(userInput), end(userInput), 
begin(userInputLower), ::tolower);
+
+             // Mis-spelling tolerance.
+            const int levDist = getLevenshteinDist(keyLower, userInputLower);
+            if (levDist > 2)
+                continue; // Not even close, keep searching.
+
+            // We found something with some differences--warn and return.
+            LOG_WRN("Incorrect JSON property [" << userInput << "]. Did you 
mean [" << key <<
+                    "] ? (Levenshtein distance: " << levDist << ")");
+
+            // Fail without exact match.
+            return false;
+        }
+
+        value = getJSONValue<T>(object, userInput);
+        if (bRemove)
+            object->remove(userInput);
+
+        LOG_TRC("Found JSON property [" << userInput << "] => [" << value << 
"]");
+        return true;
+    }
+
+    LOG_WRN("Missing JSON property [" << key << "]");
+    return false;
+}
+
+} // end namespace JsonUtil
+
+#endif
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
+
diff --git a/common/Util.hpp b/common/Util.hpp
index 2f583185..d6864589 100644
--- a/common/Util.hpp
+++ b/common/Util.hpp
@@ -10,8 +10,9 @@
 #ifndef INCLUDED_UTIL_HPP
 #define INCLUDED_UTIL_HPP
 
-#include <atomic>
 #include <cassert>
+#include <cstddef>
+#include <atomic>
 #include <functional>
 #include <memory>
 #include <mutex>
diff --git a/test/WhiteBoxTests.cpp b/test/WhiteBoxTests.cpp
index cd4b5ca4..f910aac3 100644
--- a/test/WhiteBoxTests.cpp
+++ b/test/WhiteBoxTests.cpp
@@ -19,6 +19,7 @@
 #include <Protocol.hpp>
 #include <TileDesc.hpp>
 #include <Util.hpp>
+#include <JsonUtil.hpp>
 
 /// WhiteBox unit-tests.
 class WhiteBoxTests : public CPPUNIT_NS::TestFixture
@@ -34,6 +35,7 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
     CPPUNIT_TEST(testEmptyCellCursor);
     CPPUNIT_TEST(testRectanglesIntersect);
     CPPUNIT_TEST(testAuthorization);
+    CPPUNIT_TEST(testJson);
 
     CPPUNIT_TEST_SUITE_END();
 
@@ -46,6 +48,7 @@ class WhiteBoxTests : public CPPUNIT_NS::TestFixture
     void testEmptyCellCursor();
     void testRectanglesIntersect();
     void testAuthorization();
+    void testJson();
 };
 
 void WhiteBoxTests::testLOOLProtocolFunctions()
@@ -468,6 +471,38 @@ void WhiteBoxTests::testAuthorization()
     CPPUNIT_ASSERT_EQUAL(std::string("else"), req5.get("X-Something-More"));
 }
 
+void WhiteBoxTests::testJson()
+{
+    static const char* testString =
+         
"{\"BaseFileName\":\"SomeFile.pdf\",\"DisableCopy\":true,\"DisableExport\":true,\"DisableInactiveMessages\":true,\"DisablePrint\":true,\"EnableOwnerTermination\":true,\"HideExportOption\":true,\"HidePrintOption\":true,\"OwnerId\":\"i...@owner.com\",\"PostMessageOrigin\":\"*\",\"Size\":193551,\"UserCanWrite\":true,\"UserFriendlyName\":\"Owning
 user\",\"UserId\":\"u...@user.com\",\"WatermarkText\":null}";
+
+    Poco::JSON::Object::Ptr object;
+    CPPUNIT_ASSERT(JsonUtil::parseJSON(testString, object));
+
+    size_t iValue = false;
+    JsonUtil::findJSONValue(object, "Size", iValue);
+    CPPUNIT_ASSERT_EQUAL(size_t(193551U), iValue);
+
+    bool bValue = false;
+    JsonUtil::findJSONValue(object, "DisableCopy", bValue);
+    CPPUNIT_ASSERT_EQUAL(true, bValue);
+
+    std::string sValue;
+    JsonUtil::findJSONValue(object, "BaseFileName", sValue);
+    CPPUNIT_ASSERT_EQUAL(std::string("SomeFile.pdf"), sValue);
+
+    // Don't accept inexact key names.
+    sValue.clear();
+    JsonUtil::findJSONValue(object, "basefilename", sValue);
+    CPPUNIT_ASSERT_EQUAL(std::string(), sValue);
+
+    JsonUtil::findJSONValue(object, "invalid", sValue);
+    CPPUNIT_ASSERT_EQUAL(std::string(), sValue);
+
+    JsonUtil::findJSONValue(object, "UserId", sValue);
+    CPPUNIT_ASSERT_EQUAL(std::string("u...@user.com"), sValue);
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(WhiteBoxTests);
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp
index c2f3664d..5ff0dc82 100644
--- a/wsd/Storage.cpp
+++ b/wsd/Storage.cpp
@@ -47,6 +47,7 @@
 #include <Unit.hpp>
 #include <Util.hpp>
 #include <common/FileUtil.hpp>
+#include <common/JsonUtil.hpp>
 
 bool StorageBase::FilesystemEnabled;
 bool StorageBase::WopiEnabled;
@@ -325,7 +326,8 @@ StorageBase::SaveResult 
LocalStorage::saveLocalFileToStorage(const Authorization
     return StorageBase::SaveResult(StorageBase::SaveResult::OK);
 }
 
-namespace {
+namespace
+{
 
 inline
 Poco::Net::HTTPClientSession* getHTTPClientSession(const Poco::URI& uri)
@@ -338,107 +340,6 @@ Poco::Net::HTTPClientSession* getHTTPClientSession(const 
Poco::URI& uri)
         : new Poco::Net::HTTPClientSession(uri.getHost(), uri.getPort());
 }
 
-int getLevenshteinDist(const std::string& string1, const std::string& string2) 
{
-    int matrix[string1.size() + 1][string2.size() + 1];
-    std::memset(matrix, 0, sizeof(matrix[0][0]) * (string1.size() + 1) * 
(string2.size() + 1));
-
-    for (size_t i = 0; i < string1.size() + 1; i++)
-    {
-        for (size_t j = 0; j < string2.size() + 1; j++)
-        {
-            if (i == 0)
-            {
-                matrix[i][j] = j;
-            }
-            else if (j == 0)
-            {
-                matrix[i][j] = i;
-            }
-            else if (string1[i - 1] == string2[j - 1])
-            {
-                matrix[i][j] = matrix[i - 1][j - 1];
-            }
-            else
-            {
-                matrix[i][j] = 1 + std::min(std::min(matrix[i][j - 1], 
matrix[i - 1][j]),
-                                            matrix[i - 1][j - 1]);
-            }
-        }
-    }
-
-    return matrix[string1.size()][string2.size()];
-}
-
-// Gets value for `key` directly from the given JSON in `object`
-template <typename T>
-T getJSONValue(const Poco::JSON::Object::Ptr &object, const std::string& key)
-{
-    T value = T();
-    try
-    {
-        const Poco::Dynamic::Var valueVar = object->get(key);
-        value = valueVar.convert<T>();
-    }
-    catch (const Poco::Exception& exc)
-    {
-        LOG_ERR("getJSONValue: " << exc.displayText() <<
-                (exc.nested() ? " (" + exc.nested()->displayText() + ")" : 
""));
-    }
-
-    return value;
-}
-
-// Function that searches `object` for `key` and warns if there are minor 
mis-spellings involved
-// Upon successfull search, fills `value` with value found in object.
-template <typename T>
-void getWOPIValue(const Poco::JSON::Object::Ptr &object, const std::string& 
key, T& value)
-{
-    std::vector<std::string> propertyNames;
-    object->getNames(propertyNames);
-
-    // Check each property name against given key
-    // and accept with a mis-spell tolerance of 2
-    // TODO: propertyNames can be pruned after getting its value
-    for (const auto& userInput: propertyNames)
-    {
-        std::string string1(key), string2(userInput);
-        std::transform(key.begin(), key.end(), string1.begin(), tolower);
-        std::transform(userInput.begin(), userInput.end(), string2.begin(), 
tolower);
-        int levDist = getLevenshteinDist(string1, string2);
-
-        if (levDist > 2) /* Mis-spelling tolerance */
-            continue;
-        else if (levDist > 0 || key != userInput)
-        {
-            LOG_WRN("Incorrect JSON property [" << userInput << "]. Did you 
mean " << key << " ?");
-            return;
-        }
-
-        value = getJSONValue<T>(object, userInput);
-        return;
-    }
-
-    LOG_WRN("Missing JSON property [" << key << "]");
-}
-
-// Parse the json string and fill the Poco::JSON object
-// Returns true if parsing successful otherwise false
-bool parseJSON(const std::string& json, Poco::JSON::Object::Ptr& object)
-{
-    bool success = false;
-    const size_t index = json.find_first_of('{');
-    if (index != std::string::npos)
-    {
-        const std::string stringJSON = json.substr(index);
-        Poco::JSON::Parser parser;
-        const Poco::Dynamic::Var result = parser.parse(stringJSON);
-        object = result.extract<Poco::JSON::Object::Ptr>();
-        success = true;
-    }
-
-    return success;
-}
-
 void addStorageDebugCookie(Poco::Net::HTTPRequest& request)
 {
     (void) request;
@@ -457,7 +358,7 @@ void addStorageDebugCookie(Poco::Net::HTTPRequest& request)
 #endif
 }
 
-Poco::Timestamp iso8601ToTimestamp(const std::string& iso8601Time)
+Poco::Timestamp iso8601ToTimestamp(const std::string& iso8601Time, const 
std::string& name)
 {
     Poco::Timestamp timestamp = Poco::Timestamp::fromEpochTime(0);
     try
@@ -469,8 +370,8 @@ Poco::Timestamp iso8601ToTimestamp(const std::string& 
iso8601Time)
     }
     catch (const Poco::SyntaxException& exc)
     {
-        LOG_WRN("Time [" << iso8601Time << "] is in invalid format: " << 
exc.displayText() <<
-                (exc.nested() ? " (" + exc.nested()->displayText() + ")" : 
""));
+        LOG_WRN(name << " [" << iso8601Time << "] is in invalid format: " << 
exc.displayText() <<
+                (exc.nested() ? " (" + exc.nested()->displayText() + ")" : "") 
<< ". Returning " << timestamp);
     }
 
     return timestamp;
@@ -553,27 +454,27 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> 
WopiStorage::getWOPIFileInfo(const Au
 
     LOG_DBG("WOPI::CheckFileInfo returned: " << resMsg << ". Call duration: " 
<< callDuration.count() << "s");
     Poco::JSON::Object::Ptr object;
-    if (parseJSON(resMsg, object))
+    if (JsonUtil::parseJSON(resMsg, object))
     {
-        getWOPIValue(object, "BaseFileName", filename);
-        getWOPIValue(object, "Size", size);
-        getWOPIValue(object, "OwnerId", ownerId);
-        getWOPIValue(object, "UserId", userId);
-        getWOPIValue(object, "UserFriendlyName", userName);
-        getWOPIValue(object, "UserExtraInfo", userExtraInfo);
-        getWOPIValue(object, "WatermarkText", watermarkText);
-        getWOPIValue(object, "UserCanWrite", canWrite);
-        getWOPIValue(object, "PostMessageOrigin", postMessageOrigin);
-        getWOPIValue(object, "HidePrintOption", hidePrintOption);
-        getWOPIValue(object, "HideSaveOption", hideSaveOption);
-        getWOPIValue(object, "HideExportOption", hideExportOption);
-        getWOPIValue(object, "EnableOwnerTermination", enableOwnerTermination);
-        getWOPIValue(object, "DisablePrint", disablePrint);
-        getWOPIValue(object, "DisableExport", disableExport);
-        getWOPIValue(object, "DisableCopy", disableCopy);
-        getWOPIValue(object, "DisableInactiveMessages", 
disableInactiveMessages);
-        getWOPIValue(object, "LastModifiedTime", lastModifiedTime);
-        getWOPIValue(object, "UserCanNotWriteRelative", 
userCanNotWriteRelative);
+        JsonUtil::findJSONValue(object, "BaseFileName", filename);
+        JsonUtil::findJSONValue(object, "Size", size);
+        JsonUtil::findJSONValue(object, "OwnerId", ownerId);
+        JsonUtil::findJSONValue(object, "UserId", userId);
+        JsonUtil::findJSONValue(object, "UserFriendlyName", userName);
+        JsonUtil::findJSONValue(object, "UserExtraInfo", userExtraInfo);
+        JsonUtil::findJSONValue(object, "WatermarkText", watermarkText);
+        JsonUtil::findJSONValue(object, "UserCanWrite", canWrite);
+        JsonUtil::findJSONValue(object, "PostMessageOrigin", 
postMessageOrigin);
+        JsonUtil::findJSONValue(object, "HidePrintOption", hidePrintOption);
+        JsonUtil::findJSONValue(object, "HideSaveOption", hideSaveOption);
+        JsonUtil::findJSONValue(object, "HideExportOption", hideExportOption);
+        JsonUtil::findJSONValue(object, "EnableOwnerTermination", 
enableOwnerTermination);
+        JsonUtil::findJSONValue(object, "DisablePrint", disablePrint);
+        JsonUtil::findJSONValue(object, "DisableExport", disableExport);
+        JsonUtil::findJSONValue(object, "DisableCopy", disableCopy);
+        JsonUtil::findJSONValue(object, "DisableInactiveMessages", 
disableInactiveMessages);
+        JsonUtil::findJSONValue(object, "LastModifiedTime", lastModifiedTime);
+        JsonUtil::findJSONValue(object, "UserCanNotWriteRelative", 
userCanNotWriteRelative);
     }
     else
     {
@@ -581,7 +482,7 @@ std::unique_ptr<WopiStorage::WOPIFileInfo> 
WopiStorage::getWOPIFileInfo(const Au
         throw UnauthorizedRequestException("Access denied. WOPI::CheckFileInfo 
failed on: " + uriObject.toString());
     }
 
-    const Poco::Timestamp modifiedTime = iso8601ToTimestamp(lastModifiedTime);
+    const Poco::Timestamp modifiedTime = iso8601ToTimestamp(lastModifiedTime, 
"LastModifiedTime");
     _fileInfo = FileInfo({filename, ownerId, modifiedTime, size});
 
     return std::unique_ptr<WopiStorage::WOPIFileInfo>(new 
WOPIFileInfo({userId, userName, userExtraInfo, watermarkText, canWrite, 
postMessageOrigin, hidePrintOption, hideSaveOption, hideExportOption, 
enableOwnerTermination, disablePrint, disableExport, disableCopy, 
disableInactiveMessages, userCanNotWriteRelative, callDuration}));
@@ -746,7 +647,7 @@ StorageBase::SaveResult 
WopiStorage::saveLocalFileToStorage(const Authorization&
         std::istream& rs = psession->receiveResponse(response);
         Poco::StreamCopier::copyStream(rs, oss);
 
-        std::string wopiLog(isSaveAs? "WOPI::PutRelativeFile": 
"WOPI::PutFile");
+        const std::string wopiLog(isSaveAs ? "WOPI::PutRelativeFile" : 
"WOPI::PutFile");
         LOG_INF(wopiLog << " response: " << oss.str());
         LOG_INF(wopiLog << " uploaded " << size << " bytes from [" << filePath 
<<
                 "] -> [" << uriObject.toString() << "]: " <<
@@ -756,18 +657,18 @@ StorageBase::SaveResult 
WopiStorage::saveLocalFileToStorage(const Authorization&
         {
             saveResult.setResult(StorageBase::SaveResult::OK);
             Poco::JSON::Object::Ptr object;
-            if (parseJSON(oss.str(), object))
+            if (JsonUtil::parseJSON(oss.str(), object))
             {
-                const std::string lastModifiedTime = 
getJSONValue<std::string>(object, "LastModifiedTime");
+                const std::string lastModifiedTime = 
JsonUtil::getJSONValue<std::string>(object, "LastModifiedTime");
                 LOG_TRC(wopiLog << " returns LastModifiedTime [" << 
lastModifiedTime << "].");
-                _fileInfo._modifiedTime = iso8601ToTimestamp(lastModifiedTime);
+                _fileInfo._modifiedTime = iso8601ToTimestamp(lastModifiedTime, 
"LastModifiedTime");
 
                 if (isSaveAs)
                 {
-                    const std::string name = getJSONValue<std::string>(object, 
"Name");
+                    const std::string name = 
JsonUtil::getJSONValue<std::string>(object, "Name");
                     LOG_TRC(wopiLog << " returns Name [" << name << "].");
 
-                    const std::string url = getJSONValue<std::string>(object, 
"Url");
+                    const std::string url = 
JsonUtil::getJSONValue<std::string>(object, "Url");
                     LOG_TRC(wopiLog << " returns Url [" << url << "].");
 
                     saveResult.setSaveAsResult(name, url);
@@ -794,9 +695,9 @@ StorageBase::SaveResult 
WopiStorage::saveLocalFileToStorage(const Authorization&
         {
             saveResult.setResult(StorageBase::SaveResult::CONFLICT);
             Poco::JSON::Object::Ptr object;
-            if (parseJSON(oss.str(), object))
+            if (JsonUtil::parseJSON(oss.str(), object))
             {
-                const unsigned loolStatusCode = getJSONValue<unsigned>(object, 
"LOOLStatusCode");
+                const unsigned loolStatusCode = 
JsonUtil::getJSONValue<unsigned>(object, "LOOLStatusCode");
                 if (loolStatusCode == 
static_cast<unsigned>(LOOLStatusCode::DOC_CHANGED))
                 {
                     saveResult.setResult(StorageBase::SaveResult::DOC_CHANGED);
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to