Skip to content

Commit

Permalink
WebSocket API change to make room for other connection options (SSL p…
Browse files Browse the repository at this point in the history
…inning)

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 #15334

Differential Revision: D5601675

Pulled By: javache

fbshipit-source-id: 5959d03a3e1d269b2c6775f3e0cf071ff08617bf
  • Loading branch information
rh389 authored and facebook-github-bot committed Aug 10, 2017
1 parent 0e7375a commit cd9d6e3
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 6 deletions.
4 changes: 2 additions & 2 deletions Libraries/WebSocket/RCTWebSocketModule.m
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand All @@ -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];
}];

Expand Down
19 changes: 17 additions & 2 deletions Libraries/WebSocket/WebSocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>, options: ?{origin?: string}) {
constructor(url: string, protocols: ?string | ?Array<string>, 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;
}
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")) {
Expand Down

0 comments on commit cd9d6e3

Please sign in to comment.