From cd9d6e34fdd7e4cada208ddf7ac01546756e43a3 Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Thu, 10 Aug 2017 05:59:36 -0700 Subject: [PATCH] WebSocket API change to make room for other connection options (SSL pinning) Summary: This is a simple groundwork PR to allow options to be passed to the `WebSocket` constructor. It represents a minor change to an undocumented part of the API, moving `headers` to within `options`. This will be a BC for anyone manually specifying headers other than `origin` but a) that's not a common use case with WebSockets and b) it's not documented even in code and wouldn't currently pass a flow check. NB: The third argument to the WebSocket constructor isn't part of the W3C spec, so I think this is a good place for RN-specific named parameters, better than adding a fourth argument. `protocols` needs to stay where it is, in line with the spec. If this goes through I'd like to build on it by adding an additional connection option for SSL certificate pinning, as already supported by the underlying `okhttp` and `RCTSRWebSocket`. It could later be expanded for various other uses. Currently, there's no way for a `WebSocket` user to specify any connection options other than url, protocol and headers. The fact that `WebSocket` connects in its constructor means any options have to go in there. Connect to a websocket server using iOS and Android, observe the connection headers: 1. Without specifying `origin`, the default header should be set 2. Specifying it in the old way `new WebSocket(url, protocols, { origin: 'customorigin.com' })` 3. Specifying it in the new way `new WebSocket(url, protocols, { headers: { origin: 'customorigin.com' }})`. I've tested myself using the test app with iOS and Android. Closes https://github.com/facebook/react-native/pull/15334 Differential Revision: D5601675 Pulled By: javache fbshipit-source-id: 5959d03a3e1d269b2c6775f3e0cf071ff08617bf --- Libraries/WebSocket/RCTWebSocketModule.m | 4 ++-- Libraries/WebSocket/WebSocket.js | 19 +++++++++++++++++-- .../modules/websocket/WebSocketModule.java | 6 ++++-- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/Libraries/WebSocket/RCTWebSocketModule.m b/Libraries/WebSocket/RCTWebSocketModule.m index 12f6da8a0d5e2b..6ccf3f495dca73 100644 --- a/Libraries/WebSocket/RCTWebSocketModule.m +++ b/Libraries/WebSocket/RCTWebSocketModule.m @@ -61,7 +61,7 @@ - (void)dealloc } } -RCT_EXPORT_METHOD(connect:(NSURL *)URL protocols:(NSArray *)protocols headers:(NSDictionary *)headers socketID:(nonnull NSNumber *)socketID) +RCT_EXPORT_METHOD(connect:(NSURL *)URL protocols:(NSArray *)protocols options:(NSDictionary *)options socketID:(nonnull NSNumber *)socketID) { NSMutableURLRequest *request = [NSMutableURLRequest requestWithURL:URL]; @@ -78,7 +78,7 @@ - (void)dealloc request.allHTTPHeaderFields = [NSHTTPCookie requestHeaderFieldsWithCookies:cookies]; // Load supplied headers - [headers enumerateKeysAndObjectsUsingBlock:^(NSString *key, id value, BOOL *stop) { + [options[@"headers"] enumerateKeysAndObjectsUsingBlock:^(NSString *key, id value, BOOL *stop) { [request addValue:[RCTConvert NSString:value] forHTTPHeaderField:key]; }]; diff --git a/Libraries/WebSocket/WebSocket.js b/Libraries/WebSocket/WebSocket.js index 83896e969e4684..96f159a511a346 100644 --- a/Libraries/WebSocket/WebSocket.js +++ b/Libraries/WebSocket/WebSocket.js @@ -93,12 +93,27 @@ class WebSocket extends EventTarget(...WEBSOCKET_EVENTS) { // `WebSocket.isAvailable` will return `false`, and WebSocket constructor will throw an error static isAvailable: boolean = !!WebSocketModule; - constructor(url: string, protocols: ?string | ?Array, options: ?{origin?: string}) { + constructor(url: string, protocols: ?string | ?Array, options: ?{headers?: {origin?: string}}) { super(); if (typeof protocols === 'string') { protocols = [protocols]; } + const {headers = {}, ...unrecognized} = options || {}; + + // Preserve deprecated backwards compatibility for the 'origin' option + if (unrecognized && typeof unrecognized.origin === 'string') { + console.warn('Specifying `origin` as a WebSocket connection option is deprecated. Include it under `headers` instead.'); + headers.origin = unrecognized.origin; + delete unrecognized.origin; + } + + // Warn about and discard anything else + if (Object.keys(unrecognized).length > 0) { + console.warn('Unrecognized WebSocket connection option(s) `' + Object.keys(unrecognized).join('`, `') + '`. ' + + 'Did you mean to put these under `headers`?'); + } + if (!Array.isArray(protocols)) { protocols = null; } @@ -111,7 +126,7 @@ class WebSocket extends EventTarget(...WEBSOCKET_EVENTS) { this._eventEmitter = new NativeEventEmitter(WebSocketModule); this._socketId = nextWebSocketId++; this._registerEvents(); - WebSocketModule.connect(url, protocols, options, this._socketId); + WebSocketModule.connect(url, protocols, { headers }, this._socketId); } get binaryType(): ?BinaryType { diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/websocket/WebSocketModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/websocket/WebSocketModule.java index 17df446110bf19..2bfae94e7a9428 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/websocket/WebSocketModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/websocket/WebSocketModule.java @@ -83,7 +83,7 @@ public void setContentHandler(final int id, final ContentHandler contentHandler) public void connect( final String url, @Nullable final ReadableArray protocols, - @Nullable final ReadableMap headers, + @Nullable final ReadableMap options, final int id) { OkHttpClient client = new OkHttpClient.Builder() .connectTimeout(10, TimeUnit.SECONDS) @@ -98,7 +98,9 @@ public void connect( builder.addHeader("Cookie", cookie); } - if (headers != null) { + if (options != null && options.hasKey("headers") && options.getType("headers").equals(ReadableType.Map)) { + + ReadableMap headers = options.getMap("headers"); ReadableMapKeySetIterator iterator = headers.keySetIterator(); if (!headers.hasKey("origin")) {