Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WebSocket API change to make room for other connection options (SSL pinning) #15334

Closed
wants to merge 5 commits into from
Closed

WebSocket API change to make room for other connection options (SSL pinning) #15334

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a7d4b06
Move WebSocket custom headers to options.headers, to make room for ot…
robhogan Aug 2, 2017
02fe031
Warn about deprecated WebSocket usage
rh389 Aug 3, 2017
865bfdf
Switch to single quotes
rh389 Aug 3, 2017
92099ff
Fix whitespace and flow error
robhogan Aug 3, 2017
3e1b6ea
Fix whitespace (2)
robhogan Aug 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Libraries/WebSocket/RCTWebSocketModule.m
View file
Open in desktop
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
18 changes: 16 additions & 2 deletions Libraries/WebSocket/WebSocket.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,26 @@ 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 = {}, origin, ...unrecognized } = options || {};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

property origin Property not found in object type

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm run lint doesn't have any problem with this.


// Preserve deprecated backwards compatibility for 'options.origin'
if (origin && typeof origin === 'string') {
console.warn(`Specifying "origin" as a WebSocket connection option is deprecated. Include it under "headers" instead.`);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quotes: Strings must use singlequote.

headers.origin = origin;
}

// Warn about and discard anything else
if (Object.keys(unrecognized).length > 0) {
console.warn(`Unrecognized WebSocket connection option(s) "${Object.keys(unrecognized).join(`", "`)}". `
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quotes: Strings must use singlequote.

+ `Did you mean to put these under "headers"?`);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quotes: Strings must use singlequote.

}

if (!Array.isArray(protocols)) {
protocols = null;
}
Expand All @@ -111,7 +125,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
6 changes: 4 additions & 2 deletions ReactAndroid/src/main/java/com/facebook/react/modules/websocket/WebSocketModule.java
View file
Open in desktop
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