Deprecate void* forms of StreamInterface::Read and ::Write

Updates the code to use the new interfaces

Bug: webrtc:14632
Change-Id: I33b2a25b5968de0251e3cbc84076afc013ecef6e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282680
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38601}
This commit is contained in:
Harald Alvestrand 2022-11-10 10:50:50 +00:00 committed by WebRTC LUCI CQ
parent 3d7771cc08
commit 11840ce684
7 changed files with 175 additions and 109 deletions

View file

@ -16,6 +16,7 @@
#include "absl/memory/memory.h"
#include "absl/strings/string_view.h"
#include "api/array_view.h"
#include "api/dtls_transport_interface.h"
#include "api/rtc_event_log/rtc_event_log.h"
#include "logging/rtc_event_log/events/rtc_event_dtls_transport_state.h"
@ -445,7 +446,12 @@ int DtlsTransport::SendPacket(const char* data,
return ice_transport_->SendPacket(data, size, options);
} else {
return (dtls_->WriteAll(data, size, NULL, NULL) == rtc::SR_SUCCESS)
size_t written;
int error;
return (dtls_->WriteAll(
rtc::MakeArrayView(reinterpret_cast<const uint8_t*>(data),
size),
written, error) == rtc::SR_SUCCESS)
? static_cast<int>(size)
: -1;
}
@ -691,16 +697,17 @@ void DtlsTransport::OnDtlsEvent(rtc::StreamInterface* dtls, int sig, int err) {
}
}
if (sig & rtc::SE_READ) {
char buf[kMaxDtlsPacketLen];
uint8_t buf[kMaxDtlsPacketLen];
size_t read;
int read_error;
rtc::StreamResult ret;
// The underlying DTLS stream may have received multiple DTLS records in
// one packet, so read all of them.
do {
ret = dtls_->Read(buf, sizeof(buf), &read, &read_error);
ret = dtls_->Read(buf, read, read_error);
if (ret == rtc::SR_SUCCESS) {
SignalReadPacket(this, buf, read, rtc::TimeMicros(), 0);
SignalReadPacket(this, reinterpret_cast<const char*>(buf), read,
rtc::TimeMicros(), 0);
} else if (ret == rtc::SR_EOS) {
// Remote peer shut down the association with no error.
RTC_LOG(LS_INFO) << ToString() << ": DTLS transport closed by remote";

View file

@ -28,6 +28,7 @@
#include <utility>
#include <vector>
#include "api/array_view.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
@ -215,7 +216,8 @@ static int stream_read(BIO* b, char* out, int outl) {
BIO_clear_retry_flags(b);
size_t read;
int error;
StreamResult result = stream->Read(out, outl, &read, &error);
StreamResult result = stream->Read(
rtc::MakeArrayView(reinterpret_cast<uint8_t*>(out), outl), read, error);
if (result == SR_SUCCESS) {
return checked_cast<int>(read);
} else if (result == SR_BLOCK) {
@ -232,7 +234,9 @@ static int stream_write(BIO* b, const char* in, int inl) {
BIO_clear_retry_flags(b);
size_t written;
int error;
StreamResult result = stream->Write(in, inl, &written, &error);
StreamResult result = stream->Write(
rtc::MakeArrayView(reinterpret_cast<const uint8_t*>(in), inl), written,
error);
if (result == SR_SUCCESS) {
return checked_cast<int>(written);
} else if (result == SR_BLOCK) {
@ -557,17 +561,30 @@ void OpenSSLStreamAdapter::SetInitialRetransmissionTimeout(int timeout_ms) {
//
// StreamInterface Implementation
//
// Backwards compatible Write() method using deprecated API.
// Needed because deprecated API is still =0 in API definition.
StreamResult OpenSSLStreamAdapter::Write(const void* data,
size_t data_len,
size_t* written,
int* error) {
RTC_DLOG(LS_VERBOSE) << "OpenSSLStreamAdapter::Write(" << data_len << ")";
// TODO(bugs.webrtc.org/14632): Consider doing
// RTC_CHECK_NOTREACHED(); when downstream usage is eliminated.
size_t dummy_written;
int dummy_error;
return Write(
rtc::MakeArrayView(reinterpret_cast<const uint8_t*>(data), data_len),
written ? *written : dummy_written, error ? *error : dummy_error);
}
StreamResult OpenSSLStreamAdapter::Write(rtc::ArrayView<const uint8_t> data,
size_t& written,
int& error) {
RTC_DLOG(LS_VERBOSE) << "OpenSSLStreamAdapter::Write(" << data.size() << ")";
switch (state_) {
case SSL_NONE:
// pass-through in clear text
return stream_->Write(data, data_len, written, error);
return stream_->Write(data, written, error);
case SSL_WAIT:
case SSL_CONNECTING:
@ -582,31 +599,26 @@ StreamResult OpenSSLStreamAdapter::Write(const void* data,
case SSL_ERROR:
case SSL_CLOSED:
default:
if (error) {
*error = ssl_error_code_;
}
error = ssl_error_code_;
return SR_ERROR;
}
// OpenSSL will return an error if we try to write zero bytes
if (data_len == 0) {
if (written) {
*written = 0;
}
if (data.size() == 0) {
written = 0;
return SR_SUCCESS;
}
ssl_write_needs_read_ = false;
int code = SSL_write(ssl_, data, checked_cast<int>(data_len));
int code = SSL_write(ssl_, data.data(), checked_cast<int>(data.size()));
int ssl_error = SSL_get_error(ssl_, code);
switch (ssl_error) {
case SSL_ERROR_NONE:
RTC_DLOG(LS_VERBOSE) << " -- success";
RTC_DCHECK_GT(code, 0);
RTC_DCHECK_LE(code, data_len);
if (written)
*written = code;
RTC_DCHECK_LE(code, data.size());
written = code;
return SR_SUCCESS;
case SSL_ERROR_WANT_READ:
RTC_DLOG(LS_VERBOSE) << " -- error want read";
@ -619,23 +631,33 @@ StreamResult OpenSSLStreamAdapter::Write(const void* data,
case SSL_ERROR_ZERO_RETURN:
default:
Error("SSL_write", (ssl_error ? ssl_error : -1), 0, false);
if (error) {
*error = ssl_error_code_;
}
error = ssl_error_code_;
return SR_ERROR;
}
// not reached
}
// Backwards compatible Read() method using deprecated API.
StreamResult OpenSSLStreamAdapter::Read(void* data,
size_t data_len,
size_t* read,
int* error) {
RTC_DLOG(LS_VERBOSE) << "OpenSSLStreamAdapter::Read(" << data_len << ")";
// TODO(bugs.webrtc.org/14632): Consider doing
// RTC_CHECK_NOTREACHED() when downstream usage is thought to be eliminated.
size_t dummy_read;
int dummy_error;
return Read(rtc::MakeArrayView(reinterpret_cast<uint8_t*>(data), data_len),
read ? *read : dummy_read, error ? *error : dummy_error);
}
StreamResult OpenSSLStreamAdapter::Read(rtc::ArrayView<uint8_t> data,
size_t& read,
int& error) {
RTC_DLOG(LS_VERBOSE) << "OpenSSLStreamAdapter::Read(" << data.size() << ")";
switch (state_) {
case SSL_NONE:
// pass-through in clear text
return stream_->Read(data, data_len, read, error);
return stream_->Read(data, read, error);
case SSL_WAIT:
case SSL_CONNECTING:
return SR_BLOCK;
@ -648,33 +670,27 @@ StreamResult OpenSSLStreamAdapter::Read(void* data,
return SR_EOS;
case SSL_ERROR:
default:
if (error) {
*error = ssl_error_code_;
}
error = ssl_error_code_;
return SR_ERROR;
}
// Don't trust OpenSSL with zero byte reads
if (data_len == 0) {
if (read) {
*read = 0;
}
if (data.size() == 0) {
read = 0;
return SR_SUCCESS;
}
ssl_read_needs_write_ = false;
const int code = SSL_read(ssl_, data, checked_cast<int>(data_len));
const int code = SSL_read(ssl_, data.data(), checked_cast<int>(data.size()));
const int ssl_error = SSL_get_error(ssl_, code);
switch (ssl_error) {
case SSL_ERROR_NONE:
RTC_DLOG(LS_VERBOSE) << " -- success";
RTC_DCHECK_GT(code, 0);
RTC_DCHECK_LE(code, data_len);
if (read) {
*read = code;
}
RTC_DCHECK_LE(code, data.size());
read = code;
if (ssl_mode_ == SSL_MODE_DTLS) {
// Enforce atomic reads -- this is a short read
@ -683,9 +699,7 @@ StreamResult OpenSSLStreamAdapter::Read(void* data,
if (pending) {
RTC_DLOG(LS_INFO) << " -- short DTLS read. flushing";
FlushInput(pending);
if (error) {
*error = SSE_MSG_TRUNC;
}
error = SSE_MSG_TRUNC;
return SR_ERROR;
}
}
@ -703,9 +717,7 @@ StreamResult OpenSSLStreamAdapter::Read(void* data,
return SR_EOS;
default:
Error("SSL_read", (ssl_error ? ssl_error : -1), 0, false);
if (error) {
*error = ssl_error_code_;
}
error = ssl_error_code_;
return SR_ERROR;
}
// not reached

View file

@ -95,14 +95,19 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter {
void SetMaxProtocolVersion(SSLProtocolVersion version) override;
void SetInitialRetransmissionTimeout(int timeout_ms) override;
StreamResult Read(void* data,
size_t data_len,
size_t* read,
int* error) override;
StreamResult Write(const void* data,
size_t data_len,
size_t* written,
int* error) override;
[[deprecated("bugs.webrtc.org/14632")]] StreamResult
Read(void* data, size_t data_len, size_t* read, int* error) override;
StreamResult Read(rtc::ArrayView<uint8_t> data,
size_t& read,
int& error) override;
[[deprecated("bugs.webrtc.org/14632")]] StreamResult Write(
const void* data,
size_t data_len,
size_t* written,
int* error) override;
StreamResult Write(rtc::ArrayView<const uint8_t> data,
size_t& written,
int& error) override;
void Close() override;
StreamState GetState() const override;

View file

@ -204,7 +204,9 @@ class SSLAdapterTestDummyServer : public sigslot::has_slots<> {
int error;
rtc::StreamResult r = ssl_stream_adapter_->Write(
message.data(), message.length(), &written, &error);
rtc::MakeArrayView(reinterpret_cast<const uint8_t*>(message.data()),
message.size()),
written, error);
if (r == rtc::SR_SUCCESS) {
return written;
} else {
@ -236,18 +238,19 @@ class SSLAdapterTestDummyServer : public sigslot::has_slots<> {
void OnSSLStreamAdapterEvent(rtc::StreamInterface* stream, int sig, int err) {
if (sig & rtc::SE_READ) {
char buffer[4096] = "";
uint8_t buffer[4096] = "";
size_t read;
int error;
// Read data received from the client and store it in our internal
// buffer.
rtc::StreamResult r =
stream->Read(buffer, sizeof(buffer) - 1, &read, &error);
rtc::StreamResult r = stream->Read(buffer, read, error);
if (r == rtc::SR_SUCCESS) {
buffer[read] = '\0';
RTC_LOG(LS_INFO) << "Server received '" << buffer << "'";
data_ += buffer;
// Here we assume that the buffer is interpretable as string.
char* buffer_as_char = reinterpret_cast<char*>(buffer);
RTC_LOG(LS_INFO) << "Server received '" << buffer_as_char << "'";
data_ += buffer_as_char;
}
}
}

View file

@ -17,6 +17,7 @@
#include "absl/memory/memory.h"
#include "absl/strings/string_view.h"
#include "api/array_view.h"
#include "api/task_queue/pending_task_safety_flag.h"
#include "rtc_base/buffer_queue.h"
#include "rtc_base/checks.h"
@ -165,7 +166,9 @@ class SSLDummyStreamBase : public rtc::StreamInterface,
int* error) override {
rtc::StreamResult r;
r = in_->Read(buffer, buffer_len, read, error);
r = in_->Read(
rtc::MakeArrayView(reinterpret_cast<uint8_t*>(buffer), buffer_len),
*read, *error);
if (r == rtc::SR_BLOCK)
return rtc::SR_BLOCK;
if (r == rtc::SR_EOS)
@ -201,17 +204,15 @@ class SSLDummyStreamBase : public rtc::StreamInterface,
}
// Write to the outgoing FifoBuffer
rtc::StreamResult WriteData(const void* data,
size_t data_len,
size_t* written,
int* error) {
return out_->Write(data, data_len, written, error);
rtc::StreamResult WriteData(rtc::ArrayView<const uint8_t> data,
size_t& written,
int& error) {
return out_->Write(data, written, error);
}
rtc::StreamResult Write(const void* data,
size_t data_len,
size_t* written,
int* error) override;
rtc::StreamResult Write(rtc::ArrayView<const uint8_t> data,
size_t& written,
int& error) override;
void Close() override {
RTC_LOG(LS_INFO) << "Closing outbound stream";
@ -649,17 +650,17 @@ class SSLStreamAdapterTestBase : public ::testing::Test,
rtc::StreamResult DataWritten(SSLDummyStreamBase* from,
const void* data,
size_t data_len,
size_t* written,
int* error) {
size_t& written,
int& error) {
// Randomly drop loss_ percent of packets
if (rtc::CreateRandomId() % 100 < static_cast<uint32_t>(loss_)) {
RTC_LOG(LS_VERBOSE) << "Randomly dropping packet, size=" << data_len;
*written = data_len;
written = data_len;
return rtc::SR_SUCCESS;
}
if (dtls_ && (data_len > mtu_)) {
RTC_LOG(LS_VERBOSE) << "Dropping packet > mtu, size=" << data_len;
*written = data_len;
written = data_len;
return rtc::SR_SUCCESS;
}
@ -667,17 +668,19 @@ class SSLStreamAdapterTestBase : public ::testing::Test,
// handshake packets and we damage the last byte to keep the header
// intact but break the MAC.
if (damage_ && (*static_cast<const unsigned char*>(data) == 23)) {
std::vector<char> buf(data_len);
std::vector<uint8_t> buf(data_len);
RTC_LOG(LS_VERBOSE) << "Damaging packet";
memcpy(&buf[0], data, data_len);
buf[data_len - 1]++;
return from->WriteData(&buf[0], data_len, written, error);
return from->WriteData(rtc::MakeArrayView(&buf[0], data_len), written,
error);
}
return from->WriteData(data, data_len, written, error);
return from->WriteData(
rtc::MakeArrayView(reinterpret_cast<const uint8_t*>(data), data_len),
written, error);
}
void SetDelay(int delay) { delay_ = delay; }
@ -838,7 +841,7 @@ class SSLStreamAdapterTestTLS
size_t position, tosend, size;
rtc::StreamResult rv;
size_t sent;
char block[kBlockSize];
uint8_t block[kBlockSize];
send_stream_.GetSize(&size);
if (!size)
@ -848,7 +851,8 @@ class SSLStreamAdapterTestTLS
send_stream_.GetPosition(&position);
if (send_stream_.Read(block, sizeof(block), &tosend, nullptr) !=
rtc::SR_EOS) {
rv = client_ssl_->Write(block, tosend, &sent, 0);
int error;
rv = client_ssl_->Write(rtc::MakeArrayView(block, tosend), sent, error);
if (rv == rtc::SR_SUCCESS) {
send_stream_.SetPosition(position + sent);
@ -871,13 +875,13 @@ class SSLStreamAdapterTestTLS
}
void ReadData(rtc::StreamInterface* stream) override {
char buffer[1600];
uint8_t buffer[1600];
size_t bread;
int err2;
rtc::StreamResult r;
for (;;) {
r = stream->Read(buffer, sizeof(buffer), &bread, &err2);
r = stream->Read(buffer, bread, err2);
if (r == rtc::SR_ERROR || r == rtc::SR_EOS) {
// Unfortunately, errors are the way that the stream adapter
@ -930,7 +934,7 @@ class SSLStreamAdapterTestDTLSBase : public SSLStreamAdapterTestBase {
}
void WriteData() override {
unsigned char* packet = new unsigned char[1600];
uint8_t* packet = new uint8_t[1600];
while (sent_ < count_) {
unsigned int rand_state = sent_;
@ -942,7 +946,9 @@ class SSLStreamAdapterTestDTLSBase : public SSLStreamAdapterTestBase {
}
size_t sent;
rtc::StreamResult rv = client_ssl_->Write(packet, packet_size_, &sent, 0);
int error;
rtc::StreamResult rv = client_ssl_->Write(
rtc::MakeArrayView(packet, packet_size_), sent, error);
if (rv == rtc::SR_SUCCESS) {
RTC_LOG(LS_VERBOSE) << "Sent: " << sent_;
sent_++;
@ -959,13 +965,13 @@ class SSLStreamAdapterTestDTLSBase : public SSLStreamAdapterTestBase {
}
void ReadData(rtc::StreamInterface* stream) override {
unsigned char buffer[2000];
uint8_t buffer[2000];
size_t bread;
int err2;
rtc::StreamResult r;
for (;;) {
r = stream->Read(buffer, 2000, &bread, &err2);
r = stream->Read(buffer, bread, err2);
if (r == rtc::SR_ERROR) {
// Unfortunately, errors are the way that the stream adapter
@ -1037,22 +1043,22 @@ class SSLStreamAdapterTestDTLS
: SSLStreamAdapterTestDTLSBase(cert_pem, private_key_pem) {}
};
rtc::StreamResult SSLDummyStreamBase::Write(const void* data,
size_t data_len,
size_t* written,
int* error) {
RTC_LOG(LS_VERBOSE) << "Writing to loopback " << data_len;
rtc::StreamResult SSLDummyStreamBase::Write(rtc::ArrayView<const uint8_t> data,
size_t& written,
int& error) {
RTC_LOG(LS_VERBOSE) << "Writing to loopback " << data.size();
if (first_packet_) {
first_packet_ = false;
if (test_base_->GetLoseFirstPacket()) {
RTC_LOG(LS_INFO) << "Losing initial packet of length " << data_len;
*written = data_len; // Fake successful writing also to writer.
RTC_LOG(LS_INFO) << "Losing initial packet of length " << data.size();
written = data.size(); // Fake successful writing also to writer.
return rtc::SR_SUCCESS;
}
}
return test_base_->DataWritten(this, data, data_len, written, error);
return test_base_->DataWritten(this, data.data(), data.size(), written,
error);
}
class SSLStreamAdapterTestDTLSFromPEMStrings : public SSLStreamAdapterTestDTLS {
@ -1169,15 +1175,16 @@ TEST_P(SSLStreamAdapterTestTLS, ReadWriteAfterClose) {
client_ssl_->Close();
rtc::StreamResult rv;
char block[kBlockSize];
uint8_t block[kBlockSize];
size_t dummy;
int error;
// It's an error to write after closed.
rv = client_ssl_->Write(block, sizeof(block), &dummy, nullptr);
rv = client_ssl_->Write(block, dummy, error);
ASSERT_EQ(rtc::SR_ERROR, rv);
// But after closed read gives you EOS.
rv = client_ssl_->Read(block, sizeof(block), &dummy, nullptr);
rv = client_ssl_->Read(block, dummy, error);
ASSERT_EQ(rtc::SR_EOS, rv);
}

View file

@ -15,6 +15,7 @@
#include <algorithm>
#include <string>
#include "api/array_view.h"
#include "rtc_base/checks.h"
#include "rtc_base/thread.h"
@ -31,8 +32,10 @@ StreamResult StreamInterface::WriteAll(const void* data,
StreamResult result = SR_SUCCESS;
size_t total_written = 0, current_written;
while (total_written < data_len) {
result = Write(static_cast<const char*>(data) + total_written,
data_len - total_written, &current_written, error);
result = Write(ArrayView<const uint8_t>(
reinterpret_cast<const uint8_t*>(data) + total_written,
data_len - total_written),
current_written, *error);
if (result != SR_SUCCESS)
break;
total_written += current_written;

View file

@ -69,24 +69,48 @@ class RTC_EXPORT StreamInterface {
// block, or the stream is in SS_OPENING state.
// SR_EOS: the end-of-stream has been reached, or the stream is in the
// SS_CLOSED state.
virtual StreamResult Read(void* buffer,
size_t buffer_len,
size_t* read,
int* error) = 0;
// The deprecated method has a default implementation that may be
// overridden in subclasses, rather than being =0.
// This allows subclasses to delete the method.
// TODO(bugs.webrtc.org/14632): Remove when downstream is converted.
[[deprecated("Use ArrayView version")]] virtual StreamResult
Read(void* buffer, size_t buffer_len, size_t* read, int* error) {
RTC_CHECK_NOTREACHED();
}
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
// Preserve backwards compatibility using a default implementation
// because there are subclasses
// outside of the WebRTC codebase that need to be converted.
//
// TODO(bugs.webrtc.org/14632): Remove when downstream is converted.
virtual StreamResult Read(rtc::ArrayView<uint8_t> buffer,
size_t& read,
int& error) {
return Read(buffer.data(), buffer.size(), &read, &error);
}
virtual StreamResult Write(const void* data,
size_t data_len,
size_t* written,
int* error) = 0;
#pragma clang diagnostic pop
// The deprecated method has a default implementation that may be
// overridden in subclasses, rather than being =0.
// This allows subclasses to delete the method.
// TODO(bugs.webrtc.org/14632): Remove when downstream is converted.
[[deprecated("Use ArrayView version")]] virtual StreamResult
Write(const void* data, size_t data_len, size_t* written, int* error) {
RTC_CHECK_NOTREACHED();
}
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
virtual StreamResult Write(rtc::ArrayView<const uint8_t> data,
size_t& written,
int& error) {
return Write(data.data(), data.size(), &written, &error);
}
#pragma clang diagnostic pop
// Attempt to transition to the SS_CLOSED state. SE_CLOSE will not be
// signalled as a result of this call.
virtual void Close() = 0;
@ -115,14 +139,19 @@ class RTC_EXPORT StreamInterface {
// unlike Write, the argument 'written' is always set, and may be non-zero
// on results other than SR_SUCCESS. The remaining arguments have the
// same semantics as Write.
StreamResult WriteAll(const void* data,
size_t data_len,
size_t* written,
int* error);
[[deprecated("Use version with ArrayView")]] StreamResult
WriteAll(const void* data, size_t data_len, size_t* written, int* error);
StreamResult WriteAll(ArrayView<uint8_t> data, size_t& written, int& error) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
// TODO(bugs.webrc.org/14632): Remove pragmas and change underlying
// implementation when downstream code is converted.
StreamResult WriteAll(ArrayView<const uint8_t> data,
size_t& written,
int& error) {
return WriteAll(data.data(), data.size(), &written, &error);
}
#pragma clang diagnostic pop
protected:
StreamInterface();