fix: -Wunsafe-buffer-usage warning in didRegisterForRemoteNotificationsWithDeviceToken (#44348)

* chore: move as_byte_span() to new shell/common/mac_util.h

this way it can be used by multiple mm files

* fix: -Wunsafe-buffer-usage warnings in UNNotificationResponseToNSDictionary

* refactor: use base::HexEncode() instead of rolling our own

* fixup! chore: move as_byte_span() to new shell/common/mac_util.h

* fixup! chore: move as_byte_span() to new shell/common/mac_util.h

fix: move mac_util to the right place in filenames.gni

filenames.gni

@@ -205,6 +205,8 @@ filenames = {
     "shell/common/mac/main_application_bundle.mm",
     "shell/common/mac/codesign_util.cc",
     "shell/common/mac/codesign_util.h",
+    "shell/common/mac_util.h",
+    "shell/common/mac_util.mm",
     "shell/common/node_bindings_mac.cc",
     "shell/common/node_bindings_mac.h",
     "shell/common/platform_util_mac.mm",

shell/browser/mac/electron_application_delegate.mm

@@ -10,11 +10,13 @@
 #include "base/allocator/partition_allocator/src/partition_alloc/shim/allocator_shim.h"
 #include "base/functional/callback.h"
 #include "base/mac/mac_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/sys_string_conversions.h"
 #include "shell/browser/api/electron_api_push_notifications.h"
 #include "shell/browser/browser.h"
 #include "shell/browser/mac/dict_util.h"
 #import "shell/browser/mac/electron_application.h"
+#include "shell/common/mac_util.h"
 
 #import <UserNotifications/UserNotifications.h>
 
@@ -176,18 +178,10 @@ static NSDictionary* UNNotificationResponseToNSDictionary(
 
 - (void)application:(NSApplication*)application
     didRegisterForRemoteNotificationsWithDeviceToken:(NSData*)deviceToken {
-  // https://stackoverflow.com/a/16411517
-  const char* token_data = static_cast<const char*>(deviceToken.bytes);
-  NSMutableString* token_string = [NSMutableString string];
-  for (NSUInteger i = 0; i < deviceToken.length; i++) {
-    [token_string appendFormat:@"%02.2hhX", token_data[i]];
-  }
   // Resolve outstanding APNS promises created during registration attempts
-  electron::api::PushNotifications* push_notifications =
-      electron::api::PushNotifications::Get();
-  if (push_notifications) {
+  if (auto* push_notifications = electron::api::PushNotifications::Get()) {
     push_notifications->ResolveAPNSPromiseSetWithToken(
-        base::SysNSStringToUTF8(token_string));
+        base::HexEncode(electron::util::as_byte_span(deviceToken)));
   }
 }
 

shell/common/api/electron_api_native_image_mac.mm

@@ -19,6 +19,7 @@
 #include "gin/handle.h"
 #include "shell/common/gin_converters/image_converter.h"
 #include "shell/common/gin_helper/promise.h"
+#include "shell/common/mac_util.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/image/image_skia.h"
@@ -26,19 +27,6 @@
 
 namespace electron::api {
 
-namespace {
-
-base::span<const uint8_t> as_byte_span(NSData* data) {
-  // SAFETY: There is no NSData API that passes the UNSAFE_BUFFER_USAGE
-  // test, so let's isolate the unsafe API use into this function. Instead of
-  // calling '[data bytes]' and '[data length]' directly, the rest of our
-  // code should prefer to use spans returned by this function.
-  return UNSAFE_BUFFERS(base::span{
-      reinterpret_cast<const uint8_t*>([data bytes]), [data length]});
-}
-
-}  // namespace
-
 NSData* bufferFromNSImage(NSImage* image) {
   CGImageRef ref = [image CGImageForProposedRect:nil context:nil hints:nil];
   NSBitmapImageRep* rep = [[NSBitmapImageRep alloc] initWithCGImage:ref];
@@ -140,7 +128,8 @@ gin::Handle<NativeImage> NativeImage::CreateFromNamedImage(gin::Arguments* args,
     NSData* png_data = bufferFromNSImage(image);
 
     if (args->GetNext(&hsl_shift) && hsl_shift.size() == 3) {
-      auto gfx_image = gfx::Image::CreateFrom1xPNGBytes(as_byte_span(png_data));
+      auto gfx_image = gfx::Image::CreateFrom1xPNGBytes(
+          electron::util::as_byte_span(png_data));
       color_utils::HSL shift = {safeShift(hsl_shift[0], -1),
                                 safeShift(hsl_shift[1], 0.5),
                                 safeShift(hsl_shift[2], 0.5)};
@@ -150,7 +139,8 @@ gin::Handle<NativeImage> NativeImage::CreateFromNamedImage(gin::Arguments* args,
               .AsNSImage());
     }
 
-    return CreateFromPNG(args->isolate(), as_byte_span(png_data));
+    return CreateFromPNG(args->isolate(),
+                         electron::util::as_byte_span(png_data));
   }
 }
 

shell/common/mac_util.h

@@ -0,0 +1,18 @@
+// Copyright (c) 2024 Microsoft, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ELECTRON_SHELL_MAC_UTIL_H_
+#define ELECTRON_SHELL_MAC_UTIL_H_
+
+#include "base/containers/span.h"
+
+@class NSData;
+
+namespace electron::util {
+
+base::span<const uint8_t> as_byte_span(NSData* data);
+
+}  // namespace electron::util
+
+#endif  // ELECTRON_SHELL_MAC_UTIL_H_

shell/common/mac_util.mm

@@ -0,0 +1,20 @@
+// Copyright (c) 2024 Microsoft, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/containers/span.h"
+
+namespace electron::util {
+
+base::span<const uint8_t> as_byte_span(NSData* data) {
+  // SAFETY: There is no NSData API that passes the UNSAFE_BUFFER_USAGE
+  // test, so let's isolate the unsafe API use into this function. Instead of
+  // calling '[data bytes]' and '[data length]' directly, the rest of our
+  // code should prefer to use spans returned by this function.
+  return UNSAFE_BUFFERS(base::span{
+      reinterpret_cast<const uint8_t*>([data bytes]), [data length]});
+}
+
+}  // namespace electron::util