Improve OFFER message validation

This patch improves the validation code for OFFER message handling.

Change-Id: I3967e9dfefee4b956c056ac4ab434ca850be34f8
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2446710
Reviewed-by: Jordan Bayles <jophba@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Commit-Queue: Jordan Bayles <jophba@chromium.org>

5 files changed, 109 insertions, 30 deletions

diff --git a/cast/streaming/answer_messages.cc b/cast/streaming/answer_messages.cc
index 16d99fc1..9502ac68 100644
--- a/cast/streaming/answer_messages.cc
+++ b/cast/streaming/answer_messages.cc
@@ -17,10 +17,6 @@ namespace cast {
 
 namespace {
 
-/// NOTE: Constants here are all taken from the Cast V2: Mirroring Control
-/// Protocol specification: http://goto.google.com/mirroring-control-protocol
-// TODO(jophba): document the protocol in a public repository.
-
 /// Constraint properties.
 // Audio constraints. See properties below.
 static constexpr char kAudio[] = "audio";
diff --git a/cast/streaming/capture_recommendations.h b/cast/streaming/capture_recommendations.h
index 2c8e2ac2..94855c7c 100644
--- a/cast/streaming/capture_recommendations.h
+++ b/cast/streaming/capture_recommendations.h
@@ -10,6 +10,8 @@
 #include <memory>
 #include <tuple>
 
+#include "cast/streaming/constants.h"
+
 namespace openscreen {
 namespace cast {
 
@@ -52,13 +54,9 @@ constexpr int kDefaultAudioMaxBitRate = 256 * 1000;
 constexpr BitRateLimits kDefaultAudioBitRateLimits{kDefaultAudioMinBitRate,
                                                    kDefaultAudioMaxBitRate};
 
-// Generally speaking, due to the range of human hearing (20Hz-20kHz) and the
-// Nyquist sampling theorem, 44.1kHz captures should capture all the fidelity
-// of the audio source.
-constexpr int kDefaultAudioMaxSampleRate = 44100;
-
-// Default to stereo if channel count is not provided.
-constexpr int kDefaultAudioMaxChannels = 2;
+// While generally audio should be captured at the maximum sample rate, 16kHz is
+// the recommended absolute minimum.
+constexpr int kDefaultAudioMinSampleRate = 16000;
 
 // Audio capture recommendations. Maximum delay is determined by buffer
 // constraints, and capture bit rate may vary between limits as appropriate.
@@ -72,10 +70,14 @@ struct Audio {
   std::chrono::milliseconds max_delay = kDefaultMaxDelayMs;
 
   // Represents the maximum number of audio channels.
-  int max_channels = kDefaultAudioMaxChannels;
+  int max_channels = kDefaultAudioChannels;
 
   // Represents the maximum samples per second.
-  int max_sample_rate = kDefaultAudioMaxSampleRate;
+  int max_sample_rate = kDefaultAudioSampleRate;
+
+  // Represents the absolute minimum samples per second. Generally speaking,
+  // audio should be captured at the maximum samples per second rate.
+  int min_sample_rate = kDefaultAudioMinSampleRate;
 };
 
 struct Resolution {
@@ -97,10 +99,11 @@ struct Resolution {
 
 // The minimum dimensions are as close as possible to low-definition
 // television, factoring in the receiver's aspect ratio if provided.
-constexpr Resolution kDefaultMinResolution{320, 240, 30};
+constexpr Resolution kDefaultMinResolution{kMinVideoWidth, kMinVideoHeight,
+                                           kDefaultFrameRate};
 
 // Currently mirroring only supports 1080P.
-constexpr Resolution kDefaultMaxResolution{1920, 1080, 30};
+constexpr Resolution kDefaultMaxResolution{1920, 1080, kDefaultFrameRate};
 
 // The mirroring spec suggests 300kbps as the absolute minimum bitrate.
 constexpr int kDefaultVideoMinBitRate = 300 * 1000;
diff --git a/cast/streaming/capture_recommendations_unittest.cc b/cast/streaming/capture_recommendations_unittest.cc
index a70d65b4..8e9765af 100644
--- a/cast/streaming/capture_recommendations_unittest.cc
+++ b/cast/streaming/capture_recommendations_unittest.cc
@@ -16,7 +16,7 @@ namespace capture_recommendations {
 namespace {
 
 constexpr Recommendations kDefaultRecommendations{
-    Audio{BitRateLimits{32000, 256000}, milliseconds(4000), 2, 44100},
+    Audio{BitRateLimits{32000, 256000}, milliseconds(4000), 2, 48000, 16000},
     Video{BitRateLimits{300000, 1920 * 1080 * 30}, Resolution{320, 240, 30},
           Resolution{1920, 1080, 30}, false, milliseconds(4000),
           1920 * 1080 * 30 / 8}};
@@ -224,7 +224,7 @@ TEST(CaptureRecommendationsTest, EmptyConstraints) {
 // exceeding 1080P.
 TEST(CaptureRecommendationsTest, HandlesHighEnd) {
   const Recommendations kExpected{
-      Audio{BitRateLimits{96000, 500000}, milliseconds(6000), 5, 96100},
+      Audio{BitRateLimits{96000, 500000}, milliseconds(6000), 5, 96100, 16000},
       Video{BitRateLimits{600000, 6000000}, Resolution{640, 480, 30},
             Resolution{1920, 1080, 30}, false, milliseconds(6000), 6000000}};
   Answer answer;
@@ -237,7 +237,7 @@ TEST(CaptureRecommendationsTest, HandlesHighEnd) {
 // experience.
 TEST(CaptureRecommendationsTest, HandlesLowEnd) {
   const Recommendations kExpected{
-      Audio{BitRateLimits{32000, 50000}, milliseconds(1000), 2, 22000},
+      Audio{BitRateLimits{32000, 50000}, milliseconds(1000), 2, 22000, 16000},
       Video{BitRateLimits{300000, 1000000}, Resolution{320, 240, 30},
             Resolution{1200, 800, 30}, false, milliseconds(1000), 60000}};
   Answer answer;
@@ -247,7 +247,7 @@ TEST(CaptureRecommendationsTest, HandlesLowEnd) {
 
 TEST(CaptureRecommendationsTest, HandlesTooSmallScreen) {
   const Recommendations kExpected{
-      Audio{BitRateLimits{32000, 50000}, milliseconds(1000), 2, 22000},
+      Audio{BitRateLimits{32000, 50000}, milliseconds(1000), 2, 22000, 16000},
       Video{BitRateLimits{300000, 1000000}, Resolution{320, 240, 30},
             Resolution{320, 240, 30}, false, milliseconds(1000), 60000}};
   Answer answer;
@@ -259,7 +259,7 @@ TEST(CaptureRecommendationsTest, HandlesTooSmallScreen) {
 
 TEST(CaptureRecommendationsTest, HandlesMinimumSizeScreen) {
   const Recommendations kExpected{
-      Audio{BitRateLimits{32000, 50000}, milliseconds(1000), 2, 22000},
+      Audio{BitRateLimits{32000, 50000}, milliseconds(1000), 2, 22000, 16000},
       Video{BitRateLimits{300000, 1000000}, Resolution{320, 240, 30},
             Resolution{320, 240, 30}, false, milliseconds(1000), 60000}};
   Answer answer;
@@ -271,7 +271,7 @@ TEST(CaptureRecommendationsTest, HandlesMinimumSizeScreen) {
 
 TEST(CaptureRecommendationsTest, UsesIntersectionOfDisplayAndConstraints) {
   const Recommendations kExpected{
-      Audio{BitRateLimits{96000, 500000}, milliseconds(6000), 5, 96100},
+      Audio{BitRateLimits{96000, 500000}, milliseconds(6000), 5, 96100, 16000},
       Video{BitRateLimits{600000, 6000000}, Resolution{640, 480, 30},
             // Max resolution should be 1080P, since that's the display
             // resolution. No reason to capture at 4K, even though the
diff --git a/cast/streaming/offer_messages.cc b/cast/streaming/offer_messages.cc
index 179037e6..163c0b0c 100644
--- a/cast/streaming/offer_messages.cc
+++ b/cast/streaming/offer_messages.cc
@@ -6,6 +6,7 @@
 
 #include <inttypes.h>
 
+#include <algorithm>
 #include <limits>
 #include <string>
 #include <utility>
@@ -13,6 +14,7 @@
 #include "absl/strings/match.h"
 #include "absl/strings/numbers.h"
 #include "absl/strings/str_split.h"
+#include "cast/streaming/capture_recommendations.h"
 #include "cast/streaming/constants.h"
 #include "cast/streaming/receiver_session.h"
 #include "platform/base/error.h"
@@ -55,12 +57,12 @@ ErrorOr<int> ParseRtpTimebase(const Json::Value& parent,
     return error_or_raw.error();
   }
 
+  // The spec demands a leading 1, so this isn't really a fraction.
   const auto fraction = SimpleFraction::FromString(error_or_raw.value());
-  if (fraction.is_error() || !fraction.value().is_positive()) {
+  if (fraction.is_error() || !fraction.value().is_positive() ||
+      fraction.value().numerator != 1) {
     return json::CreateParseError("RTP timebase");
   }
-  // The spec demands a leading 1, so this isn't really a fraction.
-  OSP_DCHECK(fraction.value().numerator == 1);
   return fraction.value().denominator;
 }
 
@@ -133,6 +135,12 @@ ErrorOr<Stream> ParseStream(const Json::Value& value, Stream::Type type) {
   if (!rtp_timebase) {
     return rtp_timebase.error();
   }
+  if (rtp_timebase.value() <
+          std::min(capture_recommendations::kDefaultAudioMinSampleRate,
+                   kRtpVideoTimebase) ||
+      rtp_timebase.value() > kRtpVideoTimebase) {
+    return json::CreateParameterError("rtp_timebase (sample rate)");
+  }
 
   auto target_delay = json::ParseInt(value, "targetDelay");
   std::chrono::milliseconds target_delay_ms = kDefaultTargetPlayoutDelay;
diff --git a/cast/streaming/offer_messages_unittest.cc b/cast/streaming/offer_messages_unittest.cc
index 4931927a..1a8b4783 100644
--- a/cast/streaming/offer_messages_unittest.cc
+++ b/cast/streaming/offer_messages_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "cast/streaming/offer_messages.h"
 
+#include <limits>
 #include <utility>
 
 #include "cast/streaming/rtp_defines.h"
@@ -87,7 +88,7 @@ constexpr char kValidOffer[] = R"({
 
 void ExpectFailureOnParse(absl::string_view body) {
   ErrorOr<Json::Value> root = json::Parse(body);
-  ASSERT_TRUE(root.is_value());
+  ASSERT_TRUE(root.is_value()) << root.error();
   EXPECT_TRUE(Offer::Parse(std::move(root.value())).is_error());
 }
 
@@ -194,7 +195,7 @@ TEST(OfferTest, CanParseValidButStreamlessOffer) {
     "castMode": "mirroring",
     "supportedStreams": []
   })");
-  ASSERT_TRUE(root.is_value());
+  ASSERT_TRUE(root.is_value()) << root.error();
   EXPECT_TRUE(Offer::Parse(std::move(root.value())).is_value());
 }
 
@@ -258,14 +259,85 @@ TEST(OfferTest, CanParseValidZeroBitRateAudioOffer) {
       "rtpPayloadType": 96,
       "ssrc": 19088743,
       "bitRate": 0,
-      "timeBase": "1/96000",
+      "timeBase": "1/48000",
       "channels": 5,
       "aesKey": "51029e4e2347cbcb49d57ef10177aebd",
       "aesIvMask": "7f12a19be62a36c04ae4116caaeff5d2"
     }]
   })");
-  ASSERT_TRUE(root.is_value());
-  EXPECT_TRUE(Offer::Parse(std::move(root.value())).is_value());
+  ASSERT_TRUE(root.is_value()) << root.error();
+  const auto offer = Offer::Parse(std::move(root.value()));
+  EXPECT_TRUE(offer.is_value()) << offer.error();
+}
+
+TEST(OfferTest, ErrorOnInvalidRtpTimebase) {
+  ExpectFailureOnParse(R"({
+    "castMode": "mirroring",
+    "supportedStreams": [{
+      "index": 2,
+      "type": "audio_source",
+      "codecName": "opus",
+      "rtpProfile": "cast",
+      "rtpPayloadType": 96,
+      "ssrc": 19088743,
+      "bitRate": 124000,
+      "timeBase": "1/10000000",
+      "channels": 2,
+      "aesKey": "51027e4e2347cbcb49d57ef10177aebc",
+      "aesIvMask": "7f12a19be62a36c04ae4116caaeff6d1"
+    }]
+  })");
+
+  ExpectFailureOnParse(R"({
+    "castMode": "mirroring",
+    "supportedStreams": [{
+      "index": 2,
+      "type": "audio_source",
+      "codecName": "opus",
+      "rtpProfile": "cast",
+      "rtpPayloadType": 96,
+      "ssrc": 19088743,
+      "bitRate": 124000,
+      "timeBase": "0",
+      "channels": 2,
+      "aesKey": "51027e4e2347cbcb49d57ef10177aebc",
+      "aesIvMask": "7f12a19be62a36c04ae4116caaeff6d1"
+    }]
+  })");
+
+  ExpectFailureOnParse(R"({
+    "castMode": "mirroring",
+    "supportedStreams": [{
+      "index": 2,
+      "type": "audio_source",
+      "codecName": "opus",
+      "rtpProfile": "cast",
+      "rtpPayloadType": 96,
+      "ssrc": 19088743,
+      "bitRate": 124000,
+      "timeBase": "1/1",
+      "channels": 2,
+      "aesKey": "51027e4e2347cbcb49d57ef10177aebc",
+      "aesIvMask": "7f12a19be62a36c04ae4116caaeff6d1"
+    }]
+  })");
+
+  ExpectFailureOnParse(R"({
+    "castMode": "mirroring",
+    "supportedStreams": [{
+      "index": 2,
+      "type": "audio_source",
+      "codecName": "opus",
+      "rtpProfile": "cast",
+      "rtpPayloadType": 96,
+      "ssrc": 19088743,
+      "bitRate": 124000,
+      "timeBase": "really fast plz, kthx",
+      "channels": 2,
+      "aesKey": "51027e4e2347cbcb49d57ef10177aebc",
+      "aesIvMask": "7f12a19be62a36c04ae4116caaeff6d1"
+    }]
+  })");
 }
 
 TEST(OfferTest, ErrorOnMissingVideoStreamMandatoryField) {
@@ -382,7 +454,7 @@ TEST(OfferTest, ParseAndToJsonResultsInSameOffer) {
   ExpectEqualsValidOffer(offer.value());
 
   auto eoj = offer.value().ToJson();
-  EXPECT_TRUE(eoj.is_value());
+  EXPECT_TRUE(eoj.is_value()) << eoj.error();
   ErrorOr<Offer> reparsed_offer = Offer::Parse(std::move(eoj.value()));
   ExpectEqualsValidOffer(reparsed_offer.value());
 }