Remove use of istream in RTC event log parser.
Bug: webrtc:11933,webrtc:8982
Change-Id: I8008eb704549e690d7c778f743a5b9cd0c52892c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/208941
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Elad Alon <eladalon@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33603}
diff --git a/logging/BUILD.gn b/logging/BUILD.gn
index ff98a2e..519f357 100644
--- a/logging/BUILD.gn
+++ b/logging/BUILD.gn
@@ -346,6 +346,7 @@
"../rtc_base:protobuf_utils",
"../rtc_base:rtc_base_approved",
"../rtc_base:rtc_numerics",
+ "../rtc_base/system:file_wrapper",
]
absl_deps = [
"//third_party/abseil-cpp/absl/memory",
diff --git a/logging/rtc_event_log/encoder/blob_encoding.cc b/logging/rtc_event_log/encoder/blob_encoding.cc
index 48316b0..96699dc 100644
--- a/logging/rtc_event_log/encoder/blob_encoding.cc
+++ b/logging/rtc_event_log/encoder/blob_encoding.cc
@@ -58,49 +58,30 @@
return std::vector<absl::string_view>();
}
- size_t read_idx = 0;
-
// Read the lengths of all blobs.
std::vector<uint64_t> lengths(num_of_blobs);
for (size_t i = 0; i < num_of_blobs; ++i) {
- if (read_idx >= encoded_blobs.length()) {
- RTC_DCHECK_EQ(read_idx, encoded_blobs.length());
- RTC_LOG(LS_WARNING) << "Corrupt input; excessive number of blobs.";
- return std::vector<absl::string_view>();
- }
-
- const size_t read_bytes =
- DecodeVarInt(encoded_blobs.substr(read_idx), &lengths[i]);
- if (read_bytes == 0) {
+ bool success = false;
+ std::tie(success, encoded_blobs) = DecodeVarInt(encoded_blobs, &lengths[i]);
+ if (!success) {
RTC_LOG(LS_WARNING) << "Corrupt input; varint decoding failed.";
return std::vector<absl::string_view>();
}
-
- read_idx += read_bytes;
-
- // Note: It might be that read_idx == encoded_blobs.length(), if this
- // is the last iteration, and all of the blobs are the empty string.
- RTC_DCHECK_LE(read_idx, encoded_blobs.length());
}
// Read the blobs themselves.
std::vector<absl::string_view> blobs(num_of_blobs);
for (size_t i = 0; i < num_of_blobs; ++i) {
- if (read_idx + lengths[i] < read_idx) { // Wrap-around detection.
- RTC_LOG(LS_WARNING) << "Corrupt input; unreasonably large blob sequence.";
- return std::vector<absl::string_view>();
- }
-
- if (read_idx + lengths[i] > encoded_blobs.length()) {
+ if (lengths[i] > encoded_blobs.length()) {
RTC_LOG(LS_WARNING) << "Corrupt input; blob sizes exceed input size.";
return std::vector<absl::string_view>();
}
- blobs[i] = encoded_blobs.substr(read_idx, lengths[i]);
- read_idx += lengths[i];
+ blobs[i] = encoded_blobs.substr(0, lengths[i]);
+ encoded_blobs = encoded_blobs.substr(lengths[i]);
}
- if (read_idx != encoded_blobs.length()) {
+ if (!encoded_blobs.empty()) {
RTC_LOG(LS_WARNING) << "Corrupt input; unrecognized trailer.";
return std::vector<absl::string_view>();
}
diff --git a/logging/rtc_event_log/encoder/var_int.cc b/logging/rtc_event_log/encoder/var_int.cc
index b2c695e..59c9713 100644
--- a/logging/rtc_event_log/encoder/var_int.cc
+++ b/logging/rtc_event_log/encoder/var_int.cc
@@ -39,7 +39,8 @@
// There is some code duplication between the flavors of this function.
// For performance's sake, it's best to just keep it.
-size_t DecodeVarInt(absl::string_view input, uint64_t* output) {
+std::pair<bool, absl::string_view> DecodeVarInt(absl::string_view input,
+ uint64_t* output) {
RTC_DCHECK(output);
uint64_t decoded = 0;
@@ -48,11 +49,11 @@
<< static_cast<uint64_t>(7 * i));
if (!(input[i] & 0x80)) {
*output = decoded;
- return i + 1;
+ return {true, input.substr(i + 1)};
}
}
- return 0;
+ return {false, input};
}
// There is some code duplication between the flavors of this function.
diff --git a/logging/rtc_event_log/encoder/var_int.h b/logging/rtc_event_log/encoder/var_int.h
index 178c9ce..dbe1f11 100644
--- a/logging/rtc_event_log/encoder/var_int.h
+++ b/logging/rtc_event_log/encoder/var_int.h
@@ -15,6 +15,7 @@
#include <stdint.h>
#include <string>
+#include <utility>
#include "absl/strings/string_view.h"
#include "rtc_base/bit_buffer.h"
@@ -26,20 +27,23 @@
// Encode a given uint64_t as a varint. From least to most significant,
// each batch of seven bits are put into the lower bits of a byte, and the last
// remaining bit in that byte (the highest one) marks whether additional bytes
-// follow (which happens if and only if there are other bits in |input| which
+// follow (which happens if and only if there are other bits in `input` which
// are non-zero).
// Notes: If input == 0, one byte is used. If input is uint64_t::max, exactly
// kMaxVarIntLengthBytes are used.
std::string EncodeVarInt(uint64_t input);
// Inverse of EncodeVarInt().
-// If decoding is successful, a non-zero number is returned, indicating the
-// number of bytes read from |input|, and the decoded varint is written
-// into |output|.
-// If not successful, 0 is returned, and |output| is not modified.
-size_t DecodeVarInt(absl::string_view input, uint64_t* output);
+// Returns true and the remaining (unread) slice of the input if decoding
+// succeeds. Returns false otherwise and `output` is not modified.
+std::pair<bool, absl::string_view> DecodeVarInt(absl::string_view input,
+ uint64_t* output);
// Same as other version, but uses a rtc::BitBuffer for input.
+// If decoding is successful, a non-zero number is returned, indicating the
+// number of bytes read from `input`, and the decoded varint is written
+// into `output`.
+// If not successful, 0 is returned, and `output` is not modified.
// Some bits may be consumed even if a varint fails to be read.
size_t DecodeVarInt(rtc::BitBuffer* input, uint64_t* output);
diff --git a/logging/rtc_event_log/rtc_event_log_parser.cc b/logging/rtc_event_log/rtc_event_log_parser.cc
index bdc093b..6cdaa75 100644
--- a/logging/rtc_event_log/rtc_event_log_parser.cc
+++ b/logging/rtc_event_log/rtc_event_log_parser.cc
@@ -14,8 +14,6 @@
#include <string.h>
#include <algorithm>
-#include <fstream>
-#include <istream> // no-presubmit-check TODO(webrtc:8982)
#include <limits>
#include <map>
#include <utility>
@@ -29,6 +27,7 @@
#include "logging/rtc_event_log/encoder/blob_encoding.h"
#include "logging/rtc_event_log/encoder/delta_encoding.h"
#include "logging/rtc_event_log/encoder/rtc_event_log_encoder_common.h"
+#include "logging/rtc_event_log/encoder/var_int.h"
#include "logging/rtc_event_log/rtc_event_processor.h"
#include "modules/audio_coding/audio_network_adaptor/include/audio_network_adaptor.h"
#include "modules/include/module_common_types_public.h"
@@ -42,6 +41,7 @@
#include "rtc_base/numerics/safe_conversions.h"
#include "rtc_base/numerics/sequence_number_util.h"
#include "rtc_base/protobuf_utils.h"
+#include "rtc_base/system/file_wrapper.h"
// These macros were added to convert existing code using RTC_CHECKs
// to returning a Status object instead. Macros are necessary (over
@@ -98,6 +98,8 @@
namespace webrtc {
namespace {
+constexpr int64_t kMaxLogSize = 250000000;
+
constexpr size_t kIpv4Overhead = 20;
constexpr size_t kIpv6Overhead = 40;
constexpr size_t kUdpOverhead = 8;
@@ -313,33 +315,6 @@
return VideoCodecType::kVideoCodecMultiplex;
}
-// Reads a VarInt from |stream| and returns it. Also writes the read bytes to
-// |buffer| starting |bytes_written| bytes into the buffer. |bytes_written| is
-// incremented for each written byte.
-ParsedRtcEventLog::ParseStatusOr<uint64_t> ParseVarInt(
- std::istream& stream, // no-presubmit-check TODO(webrtc:8982)
- char* buffer,
- size_t* bytes_written) {
- uint64_t varint = 0;
- for (size_t bytes_read = 0; bytes_read < 10; ++bytes_read) {
- // The most significant bit of each byte is 0 if it is the last byte in
- // the varint and 1 otherwise. Thus, we take the 7 least significant bits
- // of each byte and shift them 7 bits for each byte read previously to get
- // the (unsigned) integer.
- int byte = stream.get();
- RTC_PARSE_CHECK_OR_RETURN(!stream.eof());
- RTC_DCHECK_GE(byte, 0);
- RTC_DCHECK_LE(byte, 255);
- varint |= static_cast<uint64_t>(byte & 0x7F) << (7 * bytes_read);
- buffer[*bytes_written] = byte;
- *bytes_written += 1;
- if ((byte & 0x80) == 0) {
- return varint;
- }
- }
- RTC_PARSE_CHECK_OR_RETURN(false);
-}
-
ParsedRtcEventLog::ParseStatus GetHeaderExtensions(
std::vector<RtpExtension>* header_extensions,
const RepeatedPtrField<rtclog::RtpHeaderExtension>&
@@ -1109,27 +1084,39 @@
ParsedRtcEventLog::ParseStatus ParsedRtcEventLog::ParseFile(
const std::string& filename) {
- std::ifstream file( // no-presubmit-check TODO(webrtc:8982)
- filename, std::ios_base::in | std::ios_base::binary);
- if (!file.good() || !file.is_open()) {
- RTC_LOG(LS_WARNING) << "Could not open file for reading.";
- RTC_PARSE_CHECK_OR_RETURN(file.good() && file.is_open());
+ FileWrapper file = FileWrapper::OpenReadOnly(filename);
+ if (!file.is_open()) {
+ RTC_LOG(LS_WARNING) << "Could not open file " << filename
+ << " for reading.";
+ RTC_PARSE_CHECK_OR_RETURN(file.is_open());
}
- return ParseStream(file);
+ // Compute file size.
+ long signed_filesize = file.FileSize(); // NOLINT(runtime/int)
+ RTC_PARSE_CHECK_OR_RETURN_GE(signed_filesize, 0);
+ RTC_PARSE_CHECK_OR_RETURN_LE(signed_filesize, kMaxLogSize);
+ size_t filesize = rtc::checked_cast<size_t>(signed_filesize);
+
+ // Read file into memory.
+ std::string buffer(filesize, '\0');
+ size_t bytes_read = file.Read(&buffer[0], buffer.size());
+ if (bytes_read != filesize) {
+ RTC_LOG(LS_WARNING) << "Failed to read file " << filename;
+ RTC_PARSE_CHECK_OR_RETURN_EQ(bytes_read, filesize);
+ }
+
+ return ParseStream(buffer);
}
ParsedRtcEventLog::ParseStatus ParsedRtcEventLog::ParseString(
const std::string& s) {
- std::istringstream stream( // no-presubmit-check TODO(webrtc:8982)
- s, std::ios_base::in | std::ios_base::binary);
- return ParseStream(stream);
+ return ParseStream(s);
}
ParsedRtcEventLog::ParseStatus ParsedRtcEventLog::ParseStream(
- std::istream& stream) { // no-presubmit-check TODO(webrtc:8982)
+ const std::string& s) {
Clear();
- ParseStatus status = ParseStreamInternal(stream);
+ ParseStatus status = ParseStreamInternal(s);
// Cache the configured SSRCs.
for (const auto& video_recv_config : video_recv_configs()) {
@@ -1280,17 +1267,12 @@
}
ParsedRtcEventLog::ParseStatus ParsedRtcEventLog::ParseStreamInternal(
- std::istream& stream) { // no-presubmit-check TODO(webrtc:8982)
+ absl::string_view s) {
constexpr uint64_t kMaxEventSize = 10000000; // Sanity check.
- std::vector<char> buffer(0xFFFF);
- RTC_DCHECK(stream.good());
- while (1) {
- // Check whether we have reached end of file.
- stream.peek();
- if (stream.eof()) {
- break;
- }
+ while (!s.empty()) {
+ absl::string_view event_start = s;
+ bool success = false;
// Read the next message tag. Protobuf defines the message tag as
// (field_number << 3) | wire_type. In the legacy encoding, the field number
@@ -1298,18 +1280,18 @@
// In the new encoding we still expect the wire type to be 2, but the field
// number will be greater than 1.
constexpr uint64_t kExpectedV1Tag = (1 << 3) | 2;
- size_t bytes_written = 0;
- ParsedRtcEventLog::ParseStatusOr<uint64_t> tag =
- ParseVarInt(stream, buffer.data(), &bytes_written);
- if (!tag.ok()) {
+ uint64_t tag = 0;
+ std::tie(success, s) = DecodeVarInt(s, &tag);
+ if (!success) {
RTC_LOG(LS_WARNING)
- << "Missing field tag from beginning of protobuf event.";
+ << "Failed to read field tag from beginning of protobuf event.";
RTC_PARSE_WARN_AND_RETURN_SUCCESS_IF(allow_incomplete_logs_,
kIncompleteLogError);
- return tag.status();
+ return ParseStatus::Error("Failed to read field tag varint", __FILE__,
+ __LINE__);
}
constexpr uint64_t kWireTypeMask = 0x07;
- const uint64_t wire_type = tag.value() & kWireTypeMask;
+ const uint64_t wire_type = tag & kWireTypeMask;
if (wire_type != 2) {
RTC_LOG(LS_WARNING) << "Expected field tag with wire type 2 (length "
"delimited message). Found wire type "
@@ -1320,36 +1302,32 @@
}
// Read the length field.
- ParsedRtcEventLog::ParseStatusOr<uint64_t> message_length =
- ParseVarInt(stream, buffer.data(), &bytes_written);
- if (!message_length.ok()) {
+ uint64_t message_length = 0;
+ std::tie(success, s) = DecodeVarInt(s, &message_length);
+ if (!success) {
RTC_LOG(LS_WARNING) << "Missing message length after protobuf field tag.";
RTC_PARSE_WARN_AND_RETURN_SUCCESS_IF(allow_incomplete_logs_,
kIncompleteLogError);
- return message_length.status();
- } else if (message_length.value() > kMaxEventSize) {
+ return ParseStatus::Error("Failed to read message length varint",
+ __FILE__, __LINE__);
+ }
+
+ if (message_length > s.size()) {
RTC_LOG(LS_WARNING) << "Protobuf message length is too large.";
RTC_PARSE_WARN_AND_RETURN_SUCCESS_IF(allow_incomplete_logs_,
kIncompleteLogError);
- RTC_PARSE_CHECK_OR_RETURN_LE(message_length.value(), kMaxEventSize);
+ RTC_PARSE_CHECK_OR_RETURN_LE(message_length, kMaxEventSize);
}
- // Read the next protobuf event to a temporary char buffer.
- if (buffer.size() < bytes_written + message_length.value())
- buffer.resize(bytes_written + message_length.value());
- stream.read(buffer.data() + bytes_written, message_length.value());
- if (stream.gcount() != static_cast<int>(message_length.value())) {
- RTC_LOG(LS_WARNING) << "Failed to read protobuf message.";
- RTC_PARSE_WARN_AND_RETURN_SUCCESS_IF(allow_incomplete_logs_,
- kIncompleteLogError);
- RTC_PARSE_CHECK_OR_RETURN(false);
- }
- size_t buffer_size = bytes_written + message_length.value();
+ // Skip forward to the start of the next event.
+ s = s.substr(message_length);
+ size_t total_event_size = event_start.size() - s.size();
+ RTC_CHECK_LE(total_event_size, event_start.size());
- if (tag.value() == kExpectedV1Tag) {
+ if (tag == kExpectedV1Tag) {
// Parse the protobuf event from the buffer.
rtclog::EventStream event_stream;
- if (!event_stream.ParseFromArray(buffer.data(), buffer_size)) {
+ if (!event_stream.ParseFromArray(event_start.data(), total_event_size)) {
RTC_LOG(LS_WARNING)
<< "Failed to parse legacy-format protobuf message.";
RTC_PARSE_WARN_AND_RETURN_SUCCESS_IF(allow_incomplete_logs_,
@@ -1363,7 +1341,7 @@
} else {
// Parse the protobuf event from the buffer.
rtclog2::EventStream event_stream;
- if (!event_stream.ParseFromArray(buffer.data(), buffer_size)) {
+ if (!event_stream.ParseFromArray(event_start.data(), total_event_size)) {
RTC_LOG(LS_WARNING) << "Failed to parse new-format protobuf message.";
RTC_PARSE_WARN_AND_RETURN_SUCCESS_IF(allow_incomplete_logs_,
kIncompleteLogError);
diff --git a/logging/rtc_event_log/rtc_event_log_parser.h b/logging/rtc_event_log/rtc_event_log_parser.h
index 05ff94a..67e1a09 100644
--- a/logging/rtc_event_log/rtc_event_log_parser.h
+++ b/logging/rtc_event_log/rtc_event_log_parser.h
@@ -14,7 +14,6 @@
#include <limits>
#include <map>
#include <set>
-#include <sstream> // no-presubmit-check TODO(webrtc:8982)
#include <string>
#include <utility> // pair
#include <vector>
@@ -389,9 +388,8 @@
// Reads an RtcEventLog from a string and returns success if successful.
ParseStatus ParseString(const std::string& s);
- // Reads an RtcEventLog from an istream and returns success if successful.
- ParseStatus ParseStream(
- std::istream& stream); // no-presubmit-check TODO(webrtc:8982)
+ // Reads an RtcEventLog from an string and returns success if successful.
+ ParseStatus ParseStream(const std::string& s);
MediaType GetMediaType(uint32_t ssrc, PacketDirection direction) const;
@@ -667,8 +665,7 @@
std::vector<InferredRouteChangeEvent> GetRouteChanges() const;
private:
- ABSL_MUST_USE_RESULT ParseStatus ParseStreamInternal(
- std::istream& stream); // no-presubmit-check TODO(webrtc:8982)
+ ABSL_MUST_USE_RESULT ParseStatus ParseStreamInternal(absl::string_view s);
ABSL_MUST_USE_RESULT ParseStatus
StoreParsedLegacyEvent(const rtclog::Event& event);
