Skip to content

Commit

Permalink
ParallelBoundedReader throws std::bad_alloc (#5334)
Browse files Browse the repository at this point in the history
* Bad `hoot:status` tags shouldn't cause hoot to fail, take first status
* Catch `std::bad_alloc` in `ParallelBoundedReader` and split the envelope
* Code formatting
* Sonar fixes
  • Loading branch information
bmarchant authored Apr 24, 2022
1 parent 26bfd66 commit 176cde0
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 97 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,16 @@
* This will properly maintain the copyright information. Maxar
* copyrights will be updated automatically.
*
* @copyright Copyright (C) 2013, 2014, 2015, 2017, 2018, 2019, 2020, 2021 Maxar (http://www.maxar.com/)
* @copyright Copyright (C) 2013, 2014, 2015, 2017, 2018, 2019, 2020, 2021, 2022 Maxar (http://www.maxar.com/)
*/

// Hoot
#include <hoot/core/elements/OsmMap.h>
#include <hoot/core/TestUtils.h>
#include <hoot/core/io/OsmXmlWriter.h>
#include <hoot/core/elements/MapProjector.h>
#include <hoot/core/elements/OsmMap.h>
#include <hoot/core/io/IoUtils.h>
#include <hoot/core/io/OsmXmlWriter.h>
#include <hoot/core/ops/DuplicateNodeRemover.h>
#include <hoot/core/elements/MapProjector.h>
#include <hoot/core/util/Progress.h>

namespace hoot
Expand Down Expand Up @@ -94,9 +94,9 @@ class DuplicateNodeRemoverTest : public HootTestFixture
// hoot:* tags are metadata tags and should be ignored, so no removal here
map->clear();
NodePtr node5 = TestUtils::createNode(map);
node5->getTags().set("hoot:id", "1");
node5->getTags().set(MetadataTags::HootId(), "1");
NodePtr node6 = TestUtils::createNode(map);
node6->getTags().set("hoot:id", "2");
node6->getTags().set(MetadataTags::HootId(), "2");
MapProjector::projectToOrthographic(map);

DuplicateNodeRemover::removeNodes(map, 1.0);
Expand Down Expand Up @@ -130,10 +130,7 @@ class DuplicateNodeRemoverTest : public HootTestFixture
{
exceptionMsg = e.what();
}
CPPUNIT_ASSERT_EQUAL(
QString("Nearby node merging distance must be greater than zero. Distance specified: -1")
.toStdString(),
exceptionMsg.toStdString());
HOOT_STR_EQUALS("Nearby node merging distance must be greater than zero. Distance specified: -1", exceptionMsg);
}

void runIgnoreStatusTest()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@
* This will properly maintain the copyright information. Maxar
* copyrights will be updated automatically.
*
* @copyright Copyright (C) 2020, 2021 Maxar (http://www.maxar.com/)
* @copyright Copyright (C) 2020, 2021, 2022 Maxar (http://www.maxar.com/)
*/

// Hoot
#include <hoot/core/TestUtils.h>
#include <hoot/core/schema/OsmSchema.h>
#include <hoot/core/schema/ExactTagDifferencer.h>
#include <hoot/core/schema/MetadataTags.h>

namespace hoot
{
Expand Down Expand Up @@ -80,26 +81,26 @@ class ExactTagDifferencerTest : public HootTestFixture

NodePtr n6 = std::make_shared<Node>(Status::Unknown1, 6, 0, 0, 15.0);
Tags t6;
t6["hoot:status"] = "Unknown2";
t6[MetadataTags::HootStatus()] = "Unknown2";
n6->setTags(t6);
map->addNode(n6);

NodePtr n7 = std::make_shared<Node>(Status::Unknown1, 7, 0, 0, 15.0);
Tags t7;
t7["hoot:status"] = "Unknown2";
t7[MetadataTags::HootStatus()] = "Unknown2";
n7->setTags(t7);
map->addNode(n7);

NodePtr n8 = std::make_shared<Node>(Status::Unknown1, 8, 0, 0, 15.0);
Tags t8;
t8["hoot:status"] = "1";
t8[MetadataTags::HootStatus()] = "1";
n8->setTags(t8);
map->addNode(n8);

NodePtr n9 = std::make_shared<Node>(Status::Unknown1, 9, 0, 0, 15.0);
Tags t9;
t9["monitoring:water_level"] = "yes";
t9["hoot:status"] = "1";
t9[MetadataTags::HootStatus()] = "1";
t9["note"] = "Source 2";
t9["man_made"] = "monitoring_station";
n9->setTags(t9);
Expand Down
57 changes: 22 additions & 35 deletions hoot-core/src/main/cpp/hoot/core/elements/Status.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* This will properly maintain the copyright information. Maxar
* copyrights will be updated automatically.
*
* @copyright Copyright (C) 2017, 2018, 2019, 2021 Maxar (http://www.maxar.com/)
* @copyright Copyright (C) 2017, 2018, 2019, 2021, 2022 Maxar (http://www.maxar.com/)
*/

#include "Status.h"
Expand All @@ -39,9 +39,7 @@ int Status::getInput() const
{
int result;
if (_type > EnumEnd)
{
result = _type - EnumEnd + 1;
}
else
{
switch (_type)
Expand All @@ -53,20 +51,17 @@ int Status::getInput() const
result = 1;
break;
default:
throw HootException("This is not an input status type.");
LOG_WARN("Invalid input status type: " << _type);
result = -1;
}
}

return result;
}

QString Status::toString() const
{
if (_type > EnumEnd)
{
int inputNum = _type - EnumEnd + 2;
return QString("Input%1").arg(inputNum, 3, 10, QChar('0'));
}
return QString("Input%1").arg(_type - EnumEnd + 2, 3, 10, QChar('0'));
else
{
switch (_type)
Expand All @@ -90,9 +85,7 @@ QString Status::toString() const
QString Status::toTextStatus() const
{
if (_type > EnumEnd)
{
return toString();
}
else
{
switch (_type)
Expand Down Expand Up @@ -121,54 +114,48 @@ Status::Type Status::fromString(QString statusString)
int rawNum = statusString.toInt(&rawOk);

if (rawOk)
{
return rawNum;
}
else if (statusString == "invalid" || statusString == QString::number(Status::Invalid))

if (statusString.contains(";"))
{
return Invalid;
QStringList values = statusString.split(";");
if (values.size() > 0)
statusString = values[0];
else
return Invalid;
}
else if (statusString == "unknown1" || statusString == "input1" ||
statusString == QString::number(Status::Unknown1))
{
// Check for valid values
if (statusString == "unknown1" || statusString == "input1" || statusString == QString::number(Status::Unknown1))
return Unknown1;
}
else if (statusString == "unknown2" || statusString == "input2" ||
statusString == QString::number(Status::Unknown2))
{
else if (statusString == "unknown2" || statusString == "input2" || statusString == QString::number(Status::Unknown2))
return Unknown2;
}
else if (statusString == "conflated" || statusString == QString::number(Status::Conflated))
{
return Conflated;
}
// This is used by the DiffConflator.
else if (statusString == "tagchange" || statusString == QString::number(Status::TagChange))
{
else if (statusString == "tagchange" || statusString == QString::number(Status::TagChange)) // This is used by the DiffConflator.
return TagChange;
}
else if (statusString.startsWith("input"))
{
bool ok;
int num = statusString.replace("input", "").toInt(&ok);
if (!ok)
{
throw HootException("Invalid element type string: " + statusString);
LOG_WARN("Invalid element status string: " << statusString);
return Invalid;
}
else
{
// Input 1 and 2 are enumeration 1 and 2. After that the inputs start at EnumEnd + 1 and
// go up.
// Input 1 and 2 are enumeration 1 and 2. After that the inputs start at EnumEnd + 1 and go up.
if (num > 2)
{
num = (num - 2) + EnumEnd;
}
return num;
}
}
else if (statusString == "invalid" || statusString == QString::number(Status::Invalid))
return Invalid;
else
{
throw IllegalArgumentException("Invalid element status string: " + statusString);
LOG_WARN("Invalid element status string: " << statusString);
return Invalid;
}
}

Expand Down
15 changes: 6 additions & 9 deletions hoot-core/src/main/cpp/hoot/core/io/HootNetworkRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* This will properly maintain the copyright information. Maxar
* copyrights will be updated automatically.
*
* @copyright Copyright (C) 2018, 2019, 2020, 2021 Maxar (http://www.maxar.com/)
* @copyright Copyright (C) 2018, 2019, 2020, 2021, 2022 Maxar (http://www.maxar.com/)
*/

#include "HootNetworkRequest.h"
Expand All @@ -31,8 +31,6 @@
#include <hoot/core/io/HootNetworkCookieJar.h>
#include <hoot/core/util/ConfigOptions.h>



// Qt
#include <QEventLoop>
#include <QHostAddress>
Expand Down Expand Up @@ -123,10 +121,8 @@ bool HootNetworkRequest::_networkRequest(const QUrl& url, int timeout,
tempUrl.setUserInfo("");
}
// Add the known headers
for (QMap<QNetworkRequest::KnownHeaders, QVariant>::const_iterator it = headers.begin(); it != headers.end(); ++it)
{
for (auto it = headers.begin(); it != headers.end(); ++it)
request.setHeader(it.key(), it.value());
}
// Add any cookies if necessary
if (_cookies)
{
Expand Down Expand Up @@ -161,7 +157,8 @@ bool HootNetworkRequest::_networkRequest(const QUrl& url, int timeout,
// Start the timer
timeoutTimer.start(timeout * 1000);
// Connect the timeout lambda
QObject::connect(&timeoutTimer, &QTimer::timeout, [&]()
QObject::connect(&timeoutTimer, &QTimer::timeout,
[this, &timeoutTimer, &reply, &tempUrl, &loop]()
{
// Stop the timer first so the lambda isn't called again
timeoutTimer.stop();
Expand All @@ -177,8 +174,8 @@ bool HootNetworkRequest::_networkRequest(const QUrl& url, int timeout,
loop.exit(_status);
});
// Connect the finished lambda
// QObject::connect(pNAM.get(), &QNetworkAccessManager::finished, [&]()
QObject::connect(reply, &QNetworkReply::finished, [&]()
QObject::connect(reply, &QNetworkReply::finished,
[this, &timeoutTimer, &loop]()
{
// The finished signal is emitted when the reply is aborted in the
// timeout lambda, so don't do anything
Expand Down
12 changes: 6 additions & 6 deletions hoot-core/src/main/cpp/hoot/core/io/HootNetworkRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* This will properly maintain the copyright information. Maxar
* copyrights will be updated automatically.
*
* @copyright Copyright (C) 2018, 2019, 2020, 2021 Maxar (http://www.maxar.com/)
* @copyright Copyright (C) 2018, 2019, 2020, 2021, 2022 Maxar (http://www.maxar.com/)
*/

#ifndef HOOT_NETWORK_REQUEST_H
Expand Down Expand Up @@ -72,8 +72,8 @@ class HootNetworkRequest
* @return success
*/
bool networkRequest(const QUrl& url, int timeout,
QNetworkAccessManager::Operation http_op = QNetworkAccessManager::Operation::GetOperation,
const QByteArray& data = QByteArray());
QNetworkAccessManager::Operation http_op = QNetworkAccessManager::Operation::GetOperation,
const QByteArray& data = QByteArray());
/**
* @brief networkRequest Function to make the actual request
* @param url URL for the request
Expand All @@ -84,9 +84,9 @@ class HootNetworkRequest
* @return success
*/
bool networkRequest(const QUrl& url, int timeout,
const QMap<QNetworkRequest::KnownHeaders, QVariant>& headers,
QNetworkAccessManager::Operation http_op = QNetworkAccessManager::Operation::GetOperation,
const QByteArray& data = QByteArray());
const QMap<QNetworkRequest::KnownHeaders, QVariant>& headers,
QNetworkAccessManager::Operation http_op = QNetworkAccessManager::Operation::GetOperation,
const QByteArray& data = QByteArray());
/**
* @brief getResponseContent
* @return HTTP response content
Expand Down
8 changes: 5 additions & 3 deletions hoot-core/src/main/cpp/hoot/core/io/OsmApiReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,13 @@ bool OsmApiReader::isSupported(const QString& url) const
{
QStringList validPrefixes = supportedFormats().split(";");
const QString checkString(url.toLower());
// Check the OSM API endpoint
if (!checkString.endsWith(OsmApiEndpoints::API_PATH_MAP))
return false;
// Support HTTP and HTTPS URLs to OSM API servers
for (int i = 0; i < validPrefixes.size(); ++i)
for (const auto& prefix : qAsConst(validPrefixes))
{
if (checkString.startsWith(validPrefixes[i]) &&
checkString.endsWith(OsmApiEndpoints::API_PATH_MAP))
if (checkString.startsWith(prefix))
return true;
}
// If we fall out of loop, no dice
Expand Down
Loading

0 comments on commit 176cde0

Please sign in to comment.