Better capture the goal of TurnPortTest.TestChannelBindGetErrorResponse

Using 1 as channel_id doesn't make it clear that the goal was to
provide an invalid channel.

Bug: webrtc:345518625
Change-Id: Ie64f25b9398eafd3d0a9c8bab106e5277adef7df
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/353984
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42494}
This commit is contained in:
Mirko Bonadei 2024-06-11 16:43:50 +00:00 committed by WebRTC LUCI CQ
parent 04dd95fcac
commit 05c6e745db
5 changed files with 20 additions and 12 deletions

View File

@ -1297,8 +1297,8 @@ void TurnPort::SetCallbacksForTest(CallbacksForTest* callbacks) {
callbacks_for_test_ = callbacks;
}
bool TurnPort::SetEntryChannelId(const rtc::SocketAddress& address,
int channel_id) {
bool TurnPort::SetEntryChannelIdForTesting(const rtc::SocketAddress& address,
int channel_id) {
TurnEntry* entry = FindEntry(address);
if (!entry) {
return false;

View File

@ -192,7 +192,11 @@ class TurnPort : public Port {
}
// Finds the turn entry with `address` and sets its channel id.
// Returns true if the entry is found.
bool SetEntryChannelId(const rtc::SocketAddress& address, int channel_id);
// This method must not be used in production, it is a test only
// utility that doesn't check the channel id is valid according to
// RFC5766.
bool SetEntryChannelIdForTesting(const rtc::SocketAddress& address,
int channel_id);
void HandleConnectionDestroyed(Connection* conn) override;

View File

@ -1518,11 +1518,12 @@ TEST_F(TurnPortTest, TestChannelBindGetErrorResponse) {
ASSERT_TRUE(conn2 != nullptr);
conn1->Ping(0);
EXPECT_TRUE_SIMULATED_WAIT(conn1->writable(), kSimulatedRtt * 2, fake_clock_);
// TODO(deadbeef): SetEntryChannelId should not be a public method.
// Instead we should set an option on the fake TURN server to force it to
// send a channel bind errors.
ASSERT_TRUE(turn_port_->SetEntryChannelId(
udp_port_->Candidates()[0].address(), /*channel_id=*/1));
// TODO(bugs.webrtc.org/345518625): SetEntryChannelIdForTesting should not be
// a public method. Instead we should set an option on the fake TURN server to
// force it to send a channel bind errors.
int illegal_channel_id = kMaxTurnChannelNumber + 1u;
ASSERT_TRUE(turn_port_->SetEntryChannelIdForTesting(
udp_port_->Candidates()[0].address(), illegal_channel_id));
std::string data = "ABC";
conn1->Send(data.data(), data.length(), options);
@ -1534,6 +1535,8 @@ TEST_F(TurnPortTest, TestChannelBindGetErrorResponse) {
conn2->RegisterReceivedPacketCallback(
[&](Connection* connection, const rtc::ReceivedPacket& packet) {
// TODO(bugs.webrtc.org/345518625): Verify that the packet was
// received unchanneled, not channeled.
udp_packets_.push_back(
rtc::Buffer(packet.payload().data(), packet.payload().size()));
});

View File

@ -42,9 +42,6 @@ constexpr TimeDelta kDefaultAllocationTimeout = TimeDelta::Minutes(10);
constexpr TimeDelta kPermissionTimeout = TimeDelta::Minutes(5);
constexpr TimeDelta kChannelTimeout = TimeDelta::Minutes(10);
constexpr int kMinChannelNumber = 0x4000;
constexpr int kMaxChannelNumber = 0x7FFF;
constexpr size_t kNonceKeySize = 16;
constexpr size_t kNonceSize = 48;
@ -721,7 +718,8 @@ void TurnServerAllocation::HandleChannelBindRequest(const TurnMessage* msg) {
// Check that channel id is valid.
int channel_id = channel_attr->value() >> 16;
if (channel_id < kMinChannelNumber || channel_id > kMaxChannelNumber) {
if (channel_id < kMinTurnChannelNumber ||
channel_id > kMaxTurnChannelNumber) {
SendBadRequestResponse(msg);
return;
}

View File

@ -38,6 +38,9 @@ class PacketSocketFactory;
namespace cricket {
constexpr int kMinTurnChannelNumber = 0x4000;
constexpr int kMaxTurnChannelNumber = 0x7FFF;
class StunMessage;
class TurnMessage;
class TurnServer;