Add read support of RtpStreamId/RepairedRtpStreamId header extensions.

webrtc/modules/rtp_rtcp/source/rtp_header_extensions.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 197 matching lines @@
   RTC_DCHECK_LE(0, playout_delay.min_ms);
   RTC_DCHECK_LE(playout_delay.min_ms, playout_delay.max_ms);
   RTC_DCHECK_LE(playout_delay.max_ms, kMaxMs);
   // Convert MS to value to be sent on extension header.
   uint32_t min_delay = playout_delay.min_ms / kGranularityMs;
   uint32_t max_delay = playout_delay.max_ms / kGranularityMs;
   ByteWriter<uint32_t, 3>::WriteBigEndian(data, (min_delay << 12) | max_delay);
   return true;
 }
 
+// RtpStreamId.
+constexpr RTPExtensionType RtpStreamId::kId;
+constexpr uint8_t RtpStreamId::kValueSizeBytes;
+constexpr const char* RtpStreamId::kUri;
+
+bool RtpStreamId::Parse(rtc::ArrayView<const uint8_t> data, StreamId* rsid) {
+  if (data.empty())

[nisse-webrtc, 2017/04/10 12:25:00]

If we use nul-termination in StreamId, I think ids containing nul characters
should be rejected here. 

[danilchap, 2017/04/10 13:27:05]

General rule suggest to be strict about spec when writing, but use best-effort
when reading the data.
If some implementation of webrtc would incorrectly write value "1\0", It still
would be preferable to treat it as "1" rather than discard as invalid.

[nisse-webrtc, 2017/04/11 07:59:24]

   "Be liberal in what you accept, and
    conservative in what you send"

Let me quote a bit more of RFC 1122, Sec. 1.2.2, to give some context for this
advice, which is often misunderstood (and which I don't think recommends
acceptance of invalid data):

  Software should be written to deal with every conceivable
  error, no matter how unlikely; sooner or later a packet will
  come in with that particular combination of errors and
  attributes, and unless the software is prepared, chaos can
  ensue.
  [...]
  The second part of the principle is almost as important:
  software on other hosts may contain deficiencies that make it
  unwise to exploit legal but obscure protocol features.  It is
  unwise to stray far from the obvious and simple, lest untoward
  effects result elsewhere.

It's a bit difficult to see any security implications in this particular case,
but I think it's generally pretty bad to let different pieces of the code use
different definition of the length of the same piece of protocol data.

E.g., this version will accept a stream id of a single nul-byte (length 1), but
if that value is later interpreted as the empty string, that violates reasonable
assumptions other parts of the code can make about the stream id, since by the
spec it can never be non-empty.

[danilchap, 2017/04/11 10:05:26]

I guess that was the quote I meant.
was interesting to read the details, but there was nothing about accepting or
rejecting malformed data. Parser shouldn't crash, and both guessing data based
on invalid one [my approach] and validating and rejecting invalid data [yours]
follow the guidance.
Parser is the only one that see unparsed size. Once parsed, all the rest code
expected to use the result of the parser and shouldn't be able to see difference
between "1" and "1\0".
Good point about empty string, since empty rsid match absent of the extension,
it make sense to guarantee Parse return false in this case. 

For other cases - I have another reason to prefer current way: it keeps code
simpler.

[nisse-webrtc, 2017/04/11 10:42:53]

Fair enough. I'd prefer stricter parsing, but I don't insist. 

+    return false;
+  rsid->Set(data);
+  return true;
+}
+
+// RepairedRtpStreamId.
+constexpr RTPExtensionType RepairedRtpStreamId::kId;
+constexpr uint8_t RepairedRtpStreamId::kValueSizeBytes;
+constexpr const char* RepairedRtpStreamId::kUri;
+
+bool RepairedRtpStreamId::Parse(rtc::ArrayView<const uint8_t> data,
+                                StreamId* rsid) {
+  if (data.empty())

[nisse-webrtc, 2017/04/10 12:25:00]

And here too.

[danilchap, 2017/04/11 10:05:26]

ensured RapairedRtpStreamId::Parse implementation is same as RtpStreamId::Parse

+    return false;
+  rsid->Set(data);
+  return true;
+}
+
 }  // namespace webrtc