From aad72a533f97e8be5114e127fb681ee1596bdc89 Mon Sep 17 00:00:00 2001 From: David Pursell Date: Wed, 17 Feb 2016 10:00:09 -0800 Subject: [PATCH] fastboot: fix TCP protocol version check. Currently the TCP handshake fails if the device TCP protocol version doesn't match the host exactly, but the protocol is supposed to allow for forwards compatibility by accepting any protocol version >= itself. That way the other side can potentially lower its protocol to match and keep going. This CL fixes the protocol version check and adds corresponding unit tests. Bug: http://b/27220700 Change-Id: Ib17f0a55eb910105a27609bc94bf76a30442e92e --- fastboot/tcp.cpp | 9 ++++++--- fastboot/tcp_test.cpp | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/fastboot/tcp.cpp b/fastboot/tcp.cpp index da2880a5e..e42c4e1af 100644 --- a/fastboot/tcp.cpp +++ b/fastboot/tcp.cpp @@ -28,6 +28,7 @@ #include "tcp.h" +#include #include namespace tcp { @@ -98,7 +99,8 @@ bool TcpTransport::InitializeProtocol(std::string* error) { return false; } - char buffer[kHandshakeLength]; + char buffer[kHandshakeLength + 1]; + buffer[kHandshakeLength] = '\0'; if (socket_->ReceiveAll(buffer, kHandshakeLength, kHandshakeTimeoutMs) != kHandshakeLength) { *error = android::base::StringPrintf( "No initialization message received (%s). Target may not support TCP fastboot", @@ -111,9 +113,10 @@ bool TcpTransport::InitializeProtocol(std::string* error) { return false; } - if (memcmp(buffer + 2, "01", 2) != 0) { + int version = 0; + if (!android::base::ParseInt(buffer + 2, &version) || version < kProtocolVersion) { *error = android::base::StringPrintf("Unknown TCP protocol version %s (host version %02d)", - std::string(buffer + 2, 2).c_str(), kProtocolVersion); + buffer + 2, kProtocolVersion); return false; } diff --git a/fastboot/tcp_test.cpp b/fastboot/tcp_test.cpp index 7d80d76f5..6e867ae85 100644 --- a/fastboot/tcp_test.cpp +++ b/fastboot/tcp_test.cpp @@ -42,6 +42,16 @@ TEST(TcpConnectTest, TestSuccess) { EXPECT_EQ("", error); } +TEST(TcpConnectTest, TestNewerVersionSuccess) { + std::unique_ptr mock(new SocketMock); + mock->ExpectSend("FB01"); + mock->AddReceive("FB99"); + + std::string error; + EXPECT_NE(nullptr, tcp::internal::Connect(std::move(mock), &error)); + EXPECT_EQ("", error); +} + TEST(TcpConnectTest, TestSendFailure) { std::unique_ptr mock(new SocketMock); mock->ExpectSendFailure("FB01"); @@ -74,11 +84,11 @@ TEST(TcpConnectTest, TestBadResponseFailure) { TEST(TcpConnectTest, TestUnknownVersionFailure) { std::unique_ptr mock(new SocketMock); mock->ExpectSend("FB01"); - mock->AddReceive("FB02"); + mock->AddReceive("FB00"); std::string error; EXPECT_EQ(nullptr, tcp::internal::Connect(std::move(mock), &error)); - EXPECT_EQ("Unknown TCP protocol version 02 (host version 01)", error); + EXPECT_EQ("Unknown TCP protocol version 00 (host version 01)", error); } // Fixture to configure a SocketMock for a successful TCP connection.