[Libreoffice-commits] online.git: common/JsonUtil.hpp common/Util.hpp test/WhiteBoxTests.cpp wsd/Storage.cpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Fri Feb 23 15:54:57 UTC 2018
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.nakashian at 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 <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at 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\":\"id at owner.com\",\"PostMessageOrigin\":\"*\",\"Size\":193551,\"UserCanWrite\":true,\"UserFriendlyName\":\"Owning user\",\"UserId\":\"user at 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("user at 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);
More information about the Libreoffice-commits
mailing list