WebSocketModule uses OkHttpClientProvider#55885
Closed
cortinico wants to merge 3 commits intofacebook:mainfrom
Closed
WebSocketModule uses OkHttpClientProvider#55885cortinico wants to merge 3 commits intofacebook:mainfrom
cortinico wants to merge 3 commits intofacebook:mainfrom
Conversation
meta-cla
Bot
added
the
CLA Signed
|
@cortinico has exported this pull request. If you are a Meta employee, you can view the originating Diff in D95059116. |
cortinico
added a commit
to cortinico/react-native
that referenced
this pull request
Mar 4, 2026
Summary: Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116
cortinico
force-pushed
the
export-D95059116
branch
from
8b63f0d to
7b070af
Compare
cortinico
added a commit
to cortinico/react-native
that referenced
this pull request
Mar 4, 2026
Summary: Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116
cortinico
added a commit
to cortinico/react-native
that referenced
this pull request
Mar 4, 2026
Summary: Pull Request resolved: facebook#55885 Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116
cortinico
force-pushed
the
export-D95059116
branch
2 times, most recently
from
47911aa to
4c85df7
Compare
cortinico
added a commit
to cortinico/react-native
that referenced
this pull request
Mar 4, 2026
Summary: Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116
cortinico
force-pushed
the
export-D95059116
branch
from
4c85df7 to
64a66c5
Compare
cortinico
added a commit
to cortinico/react-native
that referenced
this pull request
Mar 4, 2026
Summary: Pull Request resolved: facebook#55885 Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116
cortinico
force-pushed
the
export-D95059116
branch
from
64a66c5 to
bcbe19e
Compare
cortinico
added a commit
to cortinico/react-native
that referenced
this pull request
Mar 4, 2026
Summary: Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116
cortinico
added a commit
to cortinico/react-native
that referenced
this pull request
Mar 4, 2026
Summary: Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116
cortinico
force-pushed
the
export-D95059116
branch
from
bcbe19e to
8eaa66b
Compare
cortinico
added a commit
to cortinico/react-native
that referenced
this pull request
Mar 4, 2026
Summary: Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116
cortinico
force-pushed
the
export-D95059116
branch
from
8eaa66b to
6af2e6d
Compare
cortinico
added a commit
to cortinico/react-native
that referenced
this pull request
Mar 4, 2026
Summary: Pull Request resolved: facebook#55885 Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116
Summary: All 7 callers of DevSupportHttpClient are within ReactAndroid, so there is no need to expose it as a public API. Make the class and all its members internal. Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D95059038
Summary: The addRequestHeader/removeRequestHeader methods don't need to live inside devsupport. Instead we can use the `setOkHttpClientFactory` to provide this same logic. Changelog: [Internal] Reviewed By: javache, cipolleschi Differential Revision: D95059080
Summary: Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116
cortinico
force-pushed
the
export-D95059116
branch
from
6af2e6d to
5a72539
Compare
|
This pull request has been merged in 8487b6d. |
zoontek
pushed a commit
to zoontek/react-native
that referenced
this pull request
Mar 9, 2026
Summary: Pull Request resolved: facebook#55885 Instead of creating a brand-new OkHttpClient.Builder() per WebSocket connection, derive from OkHttpClientProvider.getOkHttpClient() so that all connections share the same connection pool and dispatcher. Also remove the dispatcher shutdown call, since the dispatcher is now shared with the global OkHttpClient and shutting it down would kill all connections. Changelog: [Internal] Reviewed By: cipolleschi, javache Differential Revision: D95059116 fbshipit-source-id: c8052c11f26a07961bcc8e4428508abeb391541d
meta-codesync Bot
pushed a commit
that referenced
this pull request
Apr 23, 2026
…#56579) Summary: #55885 changed WebSocketModule to derive its OkHttpClient from OkHttpClientProvider.getOkHttpClient().newBuilder() in order to share the connection pool and dispatcher. The shared client carries ReactCookieJarContainer, so OkHttp's BridgeInterceptor now calls Request.Builder.header("Cookie", ...) on every outgoing request — case-insensitively replacing any Cookie / cookie header the caller passed via the WebSocket constructor's `headers` option. This silently drops caller-supplied Cookie auth on the upgrade request. Apps that rely on `new WebSocket(url, null, { headers: { cookie: ... } })` for authentication (a documented public API on Android/iOS) lose their session header on Android 0.83+, while iOS continues to work because the iOS WebSocket transport doesn't go through this interceptor pipeline. Fix: explicitly set CookieJar.NO_COOKIES on the WebSocket-derived client. ForwardingCookieHandler cookies are still added manually via getCookie(), so cookies set via WebView's CookieManager keep flowing through. The connection pool and dispatcher sharing introduced by #55885 is preserved. ## Changelog: [ANDROID] [FIXED] - WebSocketModule no longer strips a `Cookie` header passed via the WebSocket constructor's `headers` option ## Tested with: Server logs the Cookie header it receives on the upgrade request: ```js const ws = new WebSocket('wss://example.com/ws', null, { headers: { cookie: 'session=abc' }, }); ``` | Build | Server sees | | --- | --- | | 0.81.6 Android | `Cookie: session=abc` | | 0.83.6 Android (before this PR) | `Cookie: <whatever the cookie jar has, NOT session=abc>` | | 0.83.6 Android (with this PR) | `Cookie: session=abc` | | iOS, all versions | `Cookie: session=abc` | Verified locally on RN 0.83.6 + a production app whose WebSocket auth broke on the 0.83.6 upgrade — a real-time streaming feed silently downgraded logged-in users to anonymous access. Applying the same change via `WebSocketModule.setCustomClientBuilder` from the host app's MainApplication.onCreate (functionally identical to this patch) restored authenticated streaming. Verified `fetch()` and other HTTP requests still get the cookie jar's cookies correctly — only the WebSocket OkHttpClient is affected by this change. Pull Request resolved: #56579 Reviewed By: cortinico Differential Revision: D102166959 Pulled By: javache fbshipit-source-id: 72cbf66acf8ced17f6f104492de67bb6f92157ce
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Instead of creating a brand-new OkHttpClient.Builder() per WebSocket
connection, derive from OkHttpClientProvider.getOkHttpClient() so
that all connections share the same connection pool and dispatcher.
Also remove the dispatcher shutdown call, since the dispatcher is now
shared with the global OkHttpClient and shutting it down would kill
all connections.
Changelog: [Internal]
Differential Revision: D95059116