Skip to content

Commit

Permalink
Add Utils::Json::IsPositiveInteger():
Browse files Browse the repository at this point in the history
- Do not rely on is_number_unsigned() of json C++ lib, which is
  unreliable due to its design. See:
    nlohmann/json#1885
  • Loading branch information
ibc committed Dec 30, 2019
1 parent 56403ec commit 9622742
Show file tree
Hide file tree
Showing 20 changed files with 367 additions and 38 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# Changelog


### 3.4.9 (WIP)

* Add `Utils::Json::IsPositiveInteger()` to not rely on `is_number_unsigned()` of json lib, which is unreliable due to its design.
* Update Node and C++ deps.


### 3.4.8

* `libsrtp.gyp`: Fix regression in mediasoup for Windows.
Expand Down
1 change: 1 addition & 0 deletions worker/include/RTC/Parameters.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ namespace RTC
void Set(json& data);
bool HasBoolean(const std::string& key) const;
bool HasInteger(const std::string& key) const;
bool HasPositiveInteger(const std::string& key) const;
bool HasDouble(const std::string& key) const;
bool HasString(const std::string& key) const;
bool HasArrayOfIntegers(const std::string& key) const;
Expand Down
21 changes: 21 additions & 0 deletions worker/include/Utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define MS_UTILS_HPP

#include "common.hpp"
#include <json.hpp>
#include <openssl/hmac.h>
#include <cmath>
#include <cstring> // std::memcmp(), std::memcpy()
Expand All @@ -13,6 +14,8 @@
#define __builtin_popcount __popcnt
#endif

using json = nlohmann::json;

namespace Utils
{
class IP
Expand Down Expand Up @@ -363,6 +366,24 @@ namespace Utils
{
return static_cast<uint32_t>(((ms << 18) + 500) / 1000) & 0x00FFFFFF;
}

class Json
{
public:
static bool IsPositiveInteger(const json& value);
};

/* Inline static methods. */

inline bool Json::IsPositiveInteger(const json& value)
{
if (value.is_number_unsigned())
return true;
else if (value.is_number_integer())
return value.get<int64_t>() >= 0;
else
return false;
}
} // namespace Utils

#endif
1 change: 1 addition & 0 deletions worker/mediasoup-worker.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@
'test/src/RTC/RTCP/TestXr.cpp',
'test/src/Utils/TestBits.cpp',
'test/src/Utils/TestIP.cpp',
'test/src/Utils/TestJson.cpp',
'test/src/Utils/TestString.cpp',
'test/src/Utils/TestTime.cpp',
# C++ include files.
Expand Down
3 changes: 2 additions & 1 deletion worker/src/Channel/Request.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "Channel/Request.hpp"
#include "Logger.hpp"
#include "MediaSoupErrors.hpp"
#include "Utils.hpp"

namespace Channel
{
Expand Down Expand Up @@ -70,7 +71,7 @@ namespace Channel

auto jsonIdIt = jsonRequest.find("id");

if (jsonIdIt == jsonRequest.end() || !jsonIdIt->is_number_unsigned())
if (jsonIdIt == jsonRequest.end() || !Utils::Json::IsPositiveInteger(*jsonIdIt))
MS_THROW_ERROR("missing id");

this->id = jsonIdIt->get<uint32_t>();
Expand Down
12 changes: 10 additions & 2 deletions worker/src/RTC/AudioLevelObserver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "RTC/AudioLevelObserver.hpp"
#include "Logger.hpp"
#include "MediaSoupErrors.hpp"
#include "Utils.hpp"
#include "Channel/Notifier.hpp"
#include "RTC/RtpDictionaries.hpp"
#include <cmath> // std::lround()
Expand All @@ -19,8 +20,15 @@ namespace RTC

auto jsonMaxEntriesIt = data.find("maxEntries");

if (jsonMaxEntriesIt == data.end() || !jsonMaxEntriesIt->is_number_unsigned())
// clang-format off
if (
jsonMaxEntriesIt == data.end() ||
!Utils::Json::IsPositiveInteger(*jsonMaxEntriesIt)
)
// clang-format on
{
MS_THROW_TYPE_ERROR("missing maxEntries");
}

this->maxEntries = jsonMaxEntriesIt->get<uint16_t>();

Expand All @@ -39,7 +47,7 @@ namespace RTC

auto jsonIntervalIt = data.find("interval");

if (jsonIntervalIt == data.end() || !jsonIntervalIt->is_number_unsigned())
if (jsonIntervalIt == data.end() || !jsonIntervalIt->is_number())
MS_THROW_TYPE_ERROR("missing interval");

this->interval = jsonIntervalIt->get<uint16_t>();
Expand Down
9 changes: 8 additions & 1 deletion worker/src/RTC/PipeTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,15 @@ namespace RTC

auto jsonPortIt = request->data.find("port");

if (jsonPortIt == request->data.end() || !jsonPortIt->is_number_unsigned())
// clang-format off
if (
jsonPortIt == request->data.end() ||
!Utils::Json::IsPositiveInteger(*jsonPortIt)
)
// clang-format on
{
MS_THROW_TYPE_ERROR("missing port");
}

port = jsonPortIt->get<uint16_t>();

Expand Down
18 changes: 15 additions & 3 deletions worker/src/RTC/PlainRtpTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,21 +263,33 @@ namespace RTC

auto jsonPortIt = request->data.find("port");

if (jsonPortIt == request->data.end() || !jsonPortIt->is_number_unsigned())
// clang-format off
if (
jsonPortIt == request->data.end() ||
!Utils::Json::IsPositiveInteger(*jsonPortIt)
)
// clang-format on
{
MS_THROW_TYPE_ERROR("missing port");
}

port = jsonPortIt->get<uint16_t>();

auto jsonRtcpPortIt = request->data.find("rtcpPort");

if (jsonRtcpPortIt != request->data.end() && jsonRtcpPortIt->is_number_unsigned())
// clang-format off
if (
jsonRtcpPortIt != request->data.end() &&
Utils::Json::IsPositiveInteger(*jsonRtcpPortIt)
)
// clang-format on
{
if (this->rtcpMux)
MS_THROW_TYPE_ERROR("cannot set rtcpPort with rtcpMux enabled");

rtcpPort = jsonRtcpPortIt->get<uint16_t>();
}
else if (jsonRtcpPortIt == request->data.end() || !jsonRtcpPortIt->is_number_unsigned())
else
{
if (!this->rtcpMux)
MS_THROW_TYPE_ERROR("missing rtcpPort (required with rtcpMux disabled)");
Expand Down
40 changes: 35 additions & 5 deletions worker/src/RTC/Producer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,24 @@ namespace RTC

auto jsonPayloadTypeIt = codec.find("payloadType");

if (jsonPayloadTypeIt == codec.end() || !jsonPayloadTypeIt->is_number_unsigned())
// clang-format off
if (
jsonPayloadTypeIt == codec.end() ||
!Utils::Json::IsPositiveInteger(*jsonPayloadTypeIt)
)
// clang-format on
{
MS_THROW_TYPE_ERROR("wrong entry in rtpMapping.codecs (missing payloadType)");
}

auto jsonMappedPayloadTypeIt = codec.find("mappedPayloadType");

if (jsonMappedPayloadTypeIt == codec.end() || !jsonMappedPayloadTypeIt->is_number_unsigned())
// clang-format off
if (
jsonMappedPayloadTypeIt == codec.end() ||
!Utils::Json::IsPositiveInteger(*jsonMappedPayloadTypeIt)
)
// clang-format on
{
MS_THROW_TYPE_ERROR("wrong entry in rtpMapping.codecs (missing mappedPayloadType)");
}
Expand Down Expand Up @@ -111,8 +121,15 @@ namespace RTC
// ssrc is optional.
auto jsonSsrcIt = encoding.find("ssrc");

if (jsonSsrcIt != encoding.end() && jsonSsrcIt->is_number_unsigned())
// clang-format off
if (
jsonSsrcIt != encoding.end() &&
Utils::Json::IsPositiveInteger(*jsonSsrcIt)
)
// clang-format on
{
encodingMapping.ssrc = jsonSsrcIt->get<uint32_t>();
}

// rid is optional.
auto jsonRidIt = encoding.find("rid");
Expand All @@ -121,7 +138,13 @@ namespace RTC
encodingMapping.rid = jsonRidIt->get<std::string>();

// However ssrc or rid must be present (if more than 1 encoding).
if (jsonEncodingsIt->size() > 1 && jsonSsrcIt == encoding.end() && jsonRidIt == encoding.end())
// clang-format off
if (
jsonEncodingsIt->size() > 1 &&
jsonSsrcIt == encoding.end() &&
jsonRidIt == encoding.end()
)
// clang-format on
{
MS_THROW_TYPE_ERROR("wrong entry in rtpMapping.encodings (missing ssrc or rid)");
}
Expand All @@ -143,8 +166,15 @@ namespace RTC
// mappedSsrc is mandatory.
auto jsonMappedSsrcIt = encoding.find("mappedSsrc");

if (jsonMappedSsrcIt == encoding.end() || !jsonMappedSsrcIt->is_number_unsigned())
// clang-format off
if (
jsonMappedSsrcIt == encoding.end() ||
!Utils::Json::IsPositiveInteger(*jsonMappedSsrcIt)
)
// clang-format on
{
MS_THROW_TYPE_ERROR("wrong entry in rtpMapping.encodings (missing mappedSsrc)");
}

encodingMapping.mappedSsrc = jsonMappedSsrcIt->get<uint32_t>();
}
Expand Down
14 changes: 14 additions & 0 deletions worker/src/RTC/RtpDictionaries/Parameters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,20 @@ namespace RTC
return value.type == Value::Type::INTEGER;
}

bool Parameters::HasPositiveInteger(const std::string& key) const
{
MS_TRACE();

auto it = this->mapKeyValues.find(key);

if (it == this->mapKeyValues.end())
return false;

auto& value = it->second;

return value.type == Value::Type::INTEGER && value.integerValue >= 0;
}

bool Parameters::HasDouble(const std::string& key) const
{
MS_TRACE();
Expand Down
10 changes: 9 additions & 1 deletion worker/src/RTC/RtpDictionaries/RtcpParameters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "Logger.hpp"
#include "MediaSoupErrors.hpp"
#include "Utils.hpp"
#include "RTC/RtpDictionaries.hpp"

namespace RTC
Expand All @@ -25,8 +26,15 @@ namespace RTC
this->cname = jsonCnameIt->get<std::string>();

// ssrc is optional.
if (jsonSsrcIt != data.end() && jsonSsrcIt->is_number_unsigned())
// clang-format off
if (
jsonSsrcIt != data.end() &&
Utils::Json::IsPositiveInteger(*jsonSsrcIt)
)
// clang-format on
{
this->ssrc = jsonSsrcIt->get<uint32_t>();
}

// reducedSize is optional.
if (jsonRedicedSizeIt != data.end() && jsonRedicedSizeIt->is_boolean())
Expand Down
30 changes: 26 additions & 4 deletions worker/src/RTC/RtpDictionaries/RtpCodecParameters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "Logger.hpp"
#include "MediaSoupErrors.hpp"
#include "Utils.hpp"
#include "RTC/RtpDictionaries.hpp"

namespace RTC
Expand Down Expand Up @@ -32,20 +33,41 @@ namespace RTC
this->mimeType.SetMimeType(jsonMimeTypeIt->get<std::string>());

// payloadType is mandatory.
if (jsonPayloadTypeIt == data.end() || !jsonPayloadTypeIt->is_number_unsigned())
// clang-format off
if (
jsonPayloadTypeIt == data.end() ||
!Utils::Json::IsPositiveInteger(*jsonPayloadTypeIt)
)
// clang-format on
{
MS_THROW_TYPE_ERROR("missing payloadType");
}

this->payloadType = jsonPayloadTypeIt->get<uint8_t>();

// clockRate is mandatory.
if (jsonClockRateIt == data.end() || !jsonClockRateIt->is_number_unsigned())
// clang-format off
if (
jsonClockRateIt == data.end() ||
!Utils::Json::IsPositiveInteger(*jsonClockRateIt)
)
// clang-format on
{
MS_THROW_TYPE_ERROR("missing clockRate");
}

this->clockRate = jsonClockRateIt->get<uint32_t>();

// channels is optional.
if (jsonChannelsIt != data.end() && jsonChannelsIt->is_number_unsigned())
// clang-format off
if (
jsonChannelsIt != data.end() &&
Utils::Json::IsPositiveInteger(*jsonChannelsIt)
)
// clang-format on
{
this->channels = jsonChannelsIt->get<uint8_t>();
}

// parameters is optional.
if (jsonParametersIt != data.end() && jsonParametersIt->is_object())
Expand Down Expand Up @@ -114,7 +136,7 @@ namespace RTC
case RTC::RtpCodecMimeType::Subtype::RTX:
{
// A RTX codec must have 'apt' parameter.
if (!this->parameters.HasInteger(aptString))
if (!this->parameters.HasPositiveInteger(aptString))
MS_THROW_TYPE_ERROR("missing apt parameter in RTX codec");

break;
Expand Down
Loading

0 comments on commit 9622742

Please sign in to comment.