[Nearby] Set frame version and type plus small fixes

- Android expects these to be set and rejects payloads otherwise
- Fixes access of |share_target| after a std::move()
- Fixes transfer metadata process interpretation (0-1 vs 0-100)
- Fixes FourDigitString hash from auth token

Bug: None
Change-Id: I88a2570d3c5b0ee772f28944914a5ae955151511
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2371624Reviewed-by: Himanshu Jaju <himanshujaju@chromium.org>
Commit-Queue: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800961}

chrome/browser/nearby_sharing/nearby_notification_manager.cc

@@ -386,7 +386,7 @@ void NearbyNotificationManager::ShowProgress(
 
   // Show indeterminate progress while waiting for remote device to accept.
   if (transfer_metadata.status() == TransferMetadata::Status::kInProgress)
-    notification.set_progress(100.0 * transfer_metadata.progress());
+    notification.set_progress(transfer_metadata.progress());
   else
     notification.set_progress(-1);
 

chrome/browser/nearby_sharing/nearby_notification_manager_unittest.cc

@@ -237,7 +237,7 @@ TEST_F(NearbyNotificationManagerTest, ShowProgress_ShowsNotification) {
 }
 
 TEST_F(NearbyNotificationManagerTest, ShowProgress_ShowsProgress) {
-  double progress = 0.75;
+  double progress = 75.0;
 
   ShareTarget share_target;
   TransferMetadata transfer_metadata =
@@ -250,17 +250,17 @@ TEST_F(NearbyNotificationManagerTest, ShowProgress_ShowsProgress) {
   ASSERT_EQ(1u, notifications.size());
 
   const message_center::Notification& notification = notifications[0];
-  EXPECT_EQ(100.0 * progress, notification.progress());
+  EXPECT_EQ(progress, notification.progress());
 }
 
 TEST_F(NearbyNotificationManagerTest, ShowProgress_UpdatesProgress) {
   ShareTarget share_target;
   TransferMetadataBuilder transfer_metadata_builder;
-  transfer_metadata_builder.set_progress(0.75);
+  transfer_metadata_builder.set_progress(75.0);
 
   manager()->ShowProgress(share_target, transfer_metadata_builder.build());
 
-  double progress = 0.5;
+  double progress = 50.0;
   transfer_metadata_builder.set_progress(progress);
   manager()->ShowProgress(share_target, transfer_metadata_builder.build());
 
@@ -269,7 +269,7 @@ TEST_F(NearbyNotificationManagerTest, ShowProgress_UpdatesProgress) {
   ASSERT_EQ(1u, notifications.size());
 
   const message_center::Notification& notification = notifications[0];
-  EXPECT_EQ(100.0 * progress, notification.progress());
+  EXPECT_EQ(progress, notification.progress());
 }
 
 TEST_P(NearbyNotificationManagerAttachmentsTest, ShowProgress) {

chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc

@@ -109,7 +109,8 @@ base::Optional<std::string> ToFourDigitString(
   int hash = 0;
   int multiplier = 1;
   for (uint8_t byte : *bytes) {
-    hash = (hash + byte * multiplier) % kHashModulo;
+    // Java bytes are signed two's complement so cast to use the correct sign.
+    hash = (hash + static_cast<int8_t>(byte) * multiplier) % kHashModulo;
     multiplier = (multiplier * kHashBaseMultiplier) % kHashModulo;
   }
 
@@ -1644,7 +1645,10 @@ void NearbySharingServiceImpl::SendIntroduction(
 
   // Write the introduction to the remote device.
   sharing::nearby::Frame frame;
-  frame.mutable_v1()->set_allocated_introduction(introduction.release());
+  frame.set_version(sharing::nearby::Frame::V1);
+  sharing::nearby::V1Frame* v1_frame = frame.mutable_v1();
+  v1_frame->set_type(sharing::nearby::V1Frame::INTRODUCTION);
+  v1_frame->set_allocated_introduction(introduction.release());
 
   std::vector<uint8_t> data(frame.ByteSize());
   frame.SerializeToArray(data.data(), frame.ByteSize());
@@ -1763,7 +1767,9 @@ void NearbySharingServiceImpl::WriteResponse(
     NearbyConnection& connection,
     sharing::nearby::ConnectionResponseFrame::Status status) {
   sharing::nearby::Frame frame;
+  frame.set_version(sharing::nearby::Frame::V1);
   sharing::nearby::V1Frame* v1_frame = frame.mutable_v1();
+  v1_frame->set_type(sharing::nearby::V1Frame::RESPONSE);
   v1_frame->mutable_connection_response()->set_status(status);
 
   std::vector<uint8_t> data(frame.ByteSize());
@@ -2253,9 +2259,9 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse(
       std::move((*frame)->get_connection_response());
   switch (response->status) {
     case sharing::mojom::ConnectionResponseFrame::Status::kAccept: {
-      info->frames_reader()->ReadFrame(base::BindOnce(
-          &NearbySharingServiceImpl::OnFrameRead,
-          weak_ptr_factory_.GetWeakPtr(), std::move(share_target)));
+      info->frames_reader()->ReadFrame(
+          base::BindOnce(&NearbySharingServiceImpl::OnFrameRead,
+                         weak_ptr_factory_.GetWeakPtr(), share_target));
 
       info->transfer_update_callback()->OnTransferUpdate(
           share_target, TransferMetadataBuilder()

chrome/browser/nearby_sharing/paired_key_verification_runner.cc

@@ -186,7 +186,9 @@ void PairedKeyVerificationRunner::OnReadPairedKeyResultFrame(
 void PairedKeyVerificationRunner::SendPairedKeyResultFrame(
     PairedKeyVerificationResult result) {
   sharing::nearby::Frame frame;
+  frame.set_version(sharing::nearby::Frame::V1);
   sharing::nearby::V1Frame* v1_frame = frame.mutable_v1();
+  v1_frame->set_type(sharing::nearby::V1Frame::PAIRED_KEY_RESULT);
   sharing::nearby::PairedKeyResultFrame* result_frame =
       v1_frame->mutable_paired_key_result();
 
@@ -222,7 +224,9 @@ void PairedKeyVerificationRunner::SendCertificateInfo() {
     return;
 
   sharing::nearby::Frame frame;
+  frame.set_version(sharing::nearby::Frame::V1);
   sharing::nearby::V1Frame* v1_frame = frame.mutable_v1();
+  v1_frame->set_type(sharing::nearby::V1Frame::CERTIFICATE_INFO);
   sharing::nearby::CertificateInfoFrame* cert_frame =
       v1_frame->mutable_certificate_info();
   for (const auto& certificate : certificates) {
@@ -249,8 +253,11 @@ void PairedKeyVerificationRunner::SendPairedKeyEncryptionFrame() {
       certificate_manager_->GetValidPrivateCertificate(Convert(visibility_));
 
   sharing::nearby::Frame frame;
+  frame.set_version(sharing::nearby::Frame::V1);
+  sharing::nearby::V1Frame* v1_frame = frame.mutable_v1();
+  v1_frame->set_type(sharing::nearby::V1Frame::PAIRED_KEY_ENCRYPTION);
   sharing::nearby::PairedKeyEncryptionFrame* encryption_frame =
-      frame.mutable_v1()->mutable_paired_key_encryption();
+      v1_frame->mutable_paired_key_encryption();
 
   base::Optional<std::vector<uint8_t>> signature =
       private_certificate.Sign(PadPrefix(local_prefix_, raw_token_));

chrome/services/sharing/nearby/decoder/nearby_decoder.cc

@@ -243,6 +243,13 @@ void NearbySharingDecoder::DecodeFrame(const std::vector<uint8_t>& data,
     return;
   }
 
+  if (!proto_frame.has_version() ||
+      proto_frame.version() != sharing::nearby::Frame_Version_V1) {
+    LOG(ERROR) << "Invalid or missing incoming frame version";
+    std::move(callback).Run(nullptr);
+    return;
+  }
+
   if (!proto_frame.has_v1()) {
     LOG(ERROR) << "Missing incoming v1frame";
     std::move(callback).Run(nullptr);

chrome/services/sharing/nearby/decoder/nearby_decoder_unittest.cc

@@ -101,6 +101,7 @@ std::unique_ptr<sharing::nearby::Frame> BuildResponseFrame(
     sharing::nearby::ConnectionResponseFrame_Status status) {
   std::unique_ptr<sharing::nearby::Frame> frame =
       std::make_unique<sharing::nearby::Frame>();
+  frame->set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame->mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_RESPONSE);
   sharing::nearby::ConnectionResponseFrame* response =
@@ -140,6 +141,7 @@ std::unique_ptr<sharing::nearby::Frame> BuildPairedKeyResultFrame(
     sharing::nearby::PairedKeyResultFrame_Status status) {
   std::unique_ptr<sharing::nearby::Frame> frame =
       std::make_unique<sharing::nearby::Frame>();
+  frame->set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame->mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_PAIRED_KEY_RESULT);
   sharing::nearby::PairedKeyResultFrame* paired_key =
@@ -253,14 +255,22 @@ TEST_F(NearbySharingDecoderTest, InvalidDeviceNameAdvertisementDecoding) {
   decoder()->DecodeAdvertisement(v1EndpointInfo, std::move(callback));
 }
 
+TEST_F(NearbySharingDecoderTest, MissingFrameVersionDecoding) {
+  sharing::nearby::Frame frame = sharing::nearby::Frame();
+
+  ExpectNullFrame(frame);
+}
+
 TEST_F(NearbySharingDecoderTest, MissingV1FrameDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
 
   ExpectNullFrame(frame);
 }
 
 TEST_F(NearbySharingDecoderTest, V1FrameMissingTypeDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame.mutable_v1();
   v1frame->mutable_introduction();
 
@@ -269,6 +279,7 @@ TEST_F(NearbySharingDecoderTest, V1FrameMissingTypeDecoding) {
 
 TEST_F(NearbySharingDecoderTest, V1FrameMissingIntroductionFrameDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame.mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_INTRODUCTION);
 
@@ -277,6 +288,7 @@ TEST_F(NearbySharingDecoderTest, V1FrameMissingIntroductionFrameDecoding) {
 
 TEST_F(NearbySharingDecoderTest, IntroductionFrameDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame.mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_INTRODUCTION);
   sharing::nearby::IntroductionFrame* intro = v1frame->mutable_introduction();
@@ -348,6 +360,7 @@ TEST_F(NearbySharingDecoderTest, IntroductionFrameDecoding) {
 
 TEST_F(NearbySharingDecoderTest, V1FrameMissingResponseFrameDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame.mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_RESPONSE);
 
@@ -455,6 +468,7 @@ TEST_F(NearbySharingDecoderTest, ResponseFrameUnknownDecoding) {
 TEST_F(NearbySharingDecoderTest,
        V1FrameMissingPairedKeyEncryptionFrameDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame.mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_PAIRED_KEY_ENCRYPTION);
 
@@ -463,6 +477,7 @@ TEST_F(NearbySharingDecoderTest,
 
 TEST_F(NearbySharingDecoderTest, PairedKeyEncryptionFrameDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame.mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_PAIRED_KEY_ENCRYPTION);
   sharing::nearby::PairedKeyEncryptionFrame* paired_key =
@@ -486,6 +501,7 @@ TEST_F(NearbySharingDecoderTest, PairedKeyEncryptionFrameDecoding) {
 
 TEST_F(NearbySharingDecoderTest, V1FrameMissingPairedKeyResultFrameDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame.mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_PAIRED_KEY_RESULT);
 
@@ -494,6 +510,7 @@ TEST_F(NearbySharingDecoderTest, V1FrameMissingPairedKeyResultFrameDecoding) {
 
 TEST_F(NearbySharingDecoderTest, CancelFrameSuccessDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame.mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_CANCEL);
 
@@ -576,6 +593,7 @@ TEST_F(NearbySharingDecoderTest, PairedKeyResultFrameUnknownDecoding) {
 
 TEST_F(NearbySharingDecoderTest, V1FrameMissingCertificateFrameDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame.mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_CERTIFICATE_INFO);
 
@@ -584,6 +602,7 @@ TEST_F(NearbySharingDecoderTest, V1FrameMissingCertificateFrameDecoding) {
 
 TEST_F(NearbySharingDecoderTest, CertificateFrameDecoding) {
   sharing::nearby::Frame frame = sharing::nearby::Frame();
+  frame.set_version(sharing::nearby::Frame_Version_V1);
   sharing::nearby::V1Frame* v1frame = frame.mutable_v1();
   v1frame->set_type(sharing::nearby::V1Frame_FrameType_CERTIFICATE_INFO);
   sharing::nearby::CertificateInfoFrame* cert_frame =