Move URL escaping to Inspector layer

Reviewed By: Hypuk Differential Revision: D5967544 fbshipit-source-id: d741e6324aff7583778cb13c862505b61ca02a3d
swissmanu · Oct 4, 2017 · ef2e29f · ef2e29f
1 parent eae4fe8
commit ef2e29f
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 11 deletions.
diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm
@@ -328,13 +328,14 @@ - (void)start
       BOOL useCustomJSC =
         [self.delegate respondsToSelector:@selector(shouldBridgeUseCustomJSC:)] &&
         [self.delegate shouldBridgeUseCustomJSC:self];
-      NSString *escapedDeviceName = [[[UIDevice currentDevice] name] stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding];
-      NSString *escapedAppName = [[[NSBundle mainBundle] bundleIdentifier] stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding];
+      // We use the name of the device and the app for debugging & metrics
+      NSString *deviceName = [[UIDevice currentDevice] name];
+      NSString *appName = [[NSBundle mainBundle] bundleIdentifier];
       // The arg is a cache dir.  It's not used with standard JSC.
       executorFactory.reset(new JSCExecutorFactory(folly::dynamic::object
         ("OwnerIdentity", "ReactNative")
-        ("AppIdentity", [(escapedAppName ?: @"unknown") UTF8String])
-        ("DeviceIdentity", [(escapedDeviceName ?: @"unknown") UTF8String])
+        ("AppIdentity", [(appName ?: @"unknown") UTF8String])
+        ("DeviceIdentity", [(deviceName ?: @"unknown") UTF8String])
         ("UseCustomJSC", (bool)useCustomJSC)
   #if RCT_PROFILE
         ("StartSamplingProfilerOnInit", (bool)self.devSettings.startSamplingProfilerOnLaunch)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerBuilder.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerBuilder.java
@@ -6,7 +6,6 @@
 
 import android.app.Activity;
 import android.app.Application;
-import android.net.Uri;
 import android.os.Build;
 import com.facebook.infer.annotation.Assertions;
 import com.facebook.react.bridge.JSBundleLoader;
@@ -266,8 +265,9 @@ public ReactInstanceManager build() {
       mUIImplementationProvider = new UIImplementationProvider();
     }
 
-    String appName = Uri.encode(mApplication.getPackageName());
-    String deviceName = Uri.encode(getFriendlyDeviceName());
+    // We use the name of the device and the app for debugging & metrics
+    String appName = mApplication.getPackageName();
+    String deviceName = getFriendlyDeviceName();
 
     return new ReactInstanceManager(
         mApplication,

diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/systeminfo/AndroidInfoHelpers.java b/ReactAndroid/src/main/java/com/facebook/react/modules/systeminfo/AndroidInfoHelpers.java
@@ -31,7 +31,7 @@ public static String getInspectorProxyHost() {
     return getServerIpAddress(INSPECTOR_PROXY_PORT);
   }
 
-  // FIXME(festevezga): This method is duplicated in an internal module. Any changes should be applied to both.
+  // WARNING(festevezga): This RN helper method has been copied to another FB-only target. Any changes should be applied to both.
   public static String getFriendlyDeviceName() {
     if (isRunningOnGenymotion()) {
       // Genymotion already has a friendly name by default

diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp
@@ -17,6 +17,7 @@
 #include <folly/Exception.h>
 #include <folly/json.h>
 #include <folly/Memory.h>
+#include <folly/String.h>
 #include <glog/logging.h>
 #include <jschelpers/InspectorInterfaces.h>
 #include <jschelpers/JSCHelpers.h>
@@ -295,10 +296,13 @@ namespace facebook {
 #endif //defined(__ANDROID__)
       }
 
+      std::string escapedOwner = folly::uriEscape<std::string>(owner, folly::UriEscapeMode::QUERY);
+      std::string escapedApp = folly::uriEscape<std::string>(app, folly::UriEscapeMode::QUERY);
+      std::string escapedDevice = folly::uriEscape<std::string>(device, folly::UriEscapeMode::QUERY);
       std::string msg = folly::to<std::string>(
-        "GET /autoattach?title=", owner,
-        "&app=" , app,
-        "&device=" , device,
+        "GET /autoattach?title=", escapedOwner,
+        "&app=" , escapedApp,
+        "&device=" , escapedDevice,
         " HTTP/1.1\r\n\r\n");
       auto send_resp = ::send(socket_desc, msg.c_str(), msg.length(), 0);
       if (send_resp < 0) {