[Libreoffice-commits] online.git: common/Authorization.cpp common/Util.cpp common/Util.hpp
GülÅah Köse (via logerrit)
logerrit at kemper.freedesktop.org
Tue Sep 1 21:17:21 UTC 2020
common/Authorization.cpp | 33 ++++++++++++++++++++++++++++++---
common/Util.cpp | 34 +---------------------------------
common/Util.hpp | 7 -------
3 files changed, 31 insertions(+), 43 deletions(-)
New commits:
commit 845554a6a3a76dc4161409f9d6648b8ff4133068
Author: Gülşah Köse <gulsah.kose at collabora.com>
AuthorDate: Tue Sep 1 20:15:30 2020 +0300
Commit: Gülşah Köse <gulsah.kose at collabora.com>
CommitDate: Tue Sep 1 23:17:03 2020 +0200
Revert "wsd: parse headers with Poco::MessageHeader"
This reverts commit dbc562d9abc997b196fd6d4e5e71f42d442488d0.
tst-05694-05694 2020-08-26 12:59:14.343136 [ unittest ]
ERR Invalid HTTP header [def]: Malformed message:
Field name too long/no colon found| ../common/Util.cpp:980
Following part of the code tests a request with corrupted http header:
Authorization auth2(Authorization::Type::Header, "def");
Poco::Net::HTTPRequest req2;
auth2.authorizeRequest(req2);
LOK_ASSERT(!req2.has("Authorization"));
Poco library throws exception.
Change-Id: Ic31a80c0e1e325de27c23059e2bcb3f00d39ad16
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/101887
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice at gmail.com>
Reviewed-by: Gülşah Köse <gulsah.kose at collabora.com>
diff --git a/common/Authorization.cpp b/common/Authorization.cpp
index 5613aa91a..93c5704fc 100644
--- a/common/Authorization.cpp
+++ b/common/Authorization.cpp
@@ -45,15 +45,42 @@ void Authorization::authorizeRequest(Poco::Net::HTTPRequest& request) const
switch (_type)
{
case Type::Token:
- Util::setHttpHeaders(request, "Authorization: Bearer " + _data);
- assert(request.has("Authorization") && "HTTPRequest missing Authorization header");
+ request.set("Authorization", "Bearer " + _data);
break;
case Type::Header:
+ {
// there might be more headers in here; like
// Authorization: Basic ....
// X-Something-Custom: Huh
- Util::setHttpHeaders(request, _data);
+ // Split based on \n's or \r's and trim, to avoid nonsense in the
+ // headers
+ StringVector tokens(Util::tokenizeAnyOf(_data, "\n\r"));
+ for (auto it = tokens.begin(); it != tokens.end(); ++it)
+ {
+ std::string token = tokens.getParam(*it);
+
+ size_t separator = token.find_first_of(':');
+ if (separator != std::string::npos)
+ {
+ size_t headerStart = token.find_first_not_of(' ', 0);
+ size_t headerEnd = token.find_last_not_of(' ', separator - 1);
+
+ size_t valueStart = token.find_first_not_of(' ', separator + 1);
+ size_t valueEnd = token.find_last_not_of(' ');
+
+ // set the header
+ if (headerStart != std::string::npos && headerEnd != std::string::npos &&
+ valueStart != std::string::npos && valueEnd != std::string::npos)
+ {
+ size_t headerLength = headerEnd - headerStart + 1;
+ size_t valueLength = valueEnd - valueStart + 1;
+
+ request.set(token.substr(headerStart, headerLength), token.substr(valueStart, valueLength));
+ }
+ }
+ }
break;
+ }
default:
// assert(false);
throw BadRequestException("Invalid HTTP request type");
diff --git a/common/Util.cpp b/common/Util.cpp
index f1cd61b69..e0ce00250 100644
--- a/common/Util.cpp
+++ b/common/Util.cpp
@@ -59,7 +59,7 @@
#include <Poco/Util/Application.h>
#include "Common.hpp"
-#include <common/Log.hpp>
+#include "Log.hpp"
#include "Protocol.hpp"
#include "Util.hpp"
@@ -953,38 +953,6 @@ namespace Util
return std::ctime(&t);
}
- void setHttpHeaders(Poco::Net::HTTPRequest& request, const std::string& headers)
- {
- // Look for either \r or \n and replace them with a single \r\n
- // as prescribed by rfc2616 as valid header delimeter, removing
- // any invalid line breaks, to avoid nonsense in the headers.
- const StringVector tokens = Util::tokenizeAnyOf(headers, "\n\r");
- const std::string header = tokens.cat("\r\n", 0);
- try
- {
- // Now parse to preserve folded headers and other
- // corner cases that is conformant to the rfc,
- // detecting any errors and/or invalid entries.
- // NB: request.read() expects full message and will fail.
- Poco::Net::MessageHeader msgHeader;
- std::istringstream iss(header);
- msgHeader.read(iss);
- for (const auto& entry : msgHeader)
- {
- // Set each header entry.
- request.set(Util::trimmed(entry.first), Util::trimmed(entry.second));
- }
- }
- catch (const Poco::Exception& ex)
- {
- LOG_ERR("Invalid HTTP header [" << header << "]: " << ex.displayText());
- }
- catch (const std::exception& ex)
- {
- LOG_ERR("Invalid HTTP header [" << header << "]: " << ex.what());
- }
- }
-
bool isFuzzing()
{
#if LIBFUZZER
diff --git a/common/Util.hpp b/common/Util.hpp
index bb81cfb75..9dbfebe8b 100644
--- a/common/Util.hpp
+++ b/common/Util.hpp
@@ -35,7 +35,6 @@
#include <Poco/File.h>
#include <Poco/Path.h>
#include <Poco/RegularExpression.h>
-#include <Poco/Net/HTTPRequest.h>
#define LOK_USE_UNSTABLE_API
#include <LibreOfficeKit/LibreOfficeKitEnums.h>
@@ -1083,12 +1082,6 @@ int main(int argc, char**argv)
/// conversion from steady_clock for debugging / tracing
std::string getSteadyClockAsString(const std::chrono::steady_clock::time_point &time);
- /// Set the request header by splitting multiple entries by \r or \n.
- /// Needed to sanitize user-provided http headers, after decoding.
- /// This is much more tolerant to line breaks than the rfc allows.
- /// Note: probably should move to a more appropriate home.
- void setHttpHeaders(Poco::Net::HTTPRequest& request, const std::string& headers);
-
/// Automatically execute code at end of current scope.
/// Used for exception-safe code.
class ScopeGuard
More information about the Libreoffice-commits
mailing list