Summary:
^
Note: this is already a working case. The difference is that if we are unable to establish a socket connection, we will not attempt to create one using rsocket.
Changelog: Removes rsocket-fallback for mobile clients
Reviewed By: nikoant
Differential Revision: D33655430
fbshipit-source-id: cb6f752f2d1354ab46d011b1f19c89520e1e7dd3
Summary:
The problem seems to be exclusive to Android emulators.
Bringing the emulator ON from a warm state brings the app from is previous state before shutting down. At this point there may have been a Flipper connection.
Flipper tries to connect and does so successfully replacing the previously, now dead connection.
The problem is that replacing that connection triggers the reconnect cycle as Flipper thinks the connection is dead and hence the reconnect loop initiates.
Changelog: Release an existing client before attempting a secure connection
Reviewed By: passy
Differential Revision: D34080726
fbshipit-source-id: 8185adb492dd4d9255fcea5874ca2e5b7fee0c84
Summary:
Changelog: Fallback to RSockets if cert exchange fails even if the connection was successful.
Currently, we fallback to RSockets only if the connected is rejected.
It turns out that WS server does not reject connection from our RSocket client. Instead, it rejects the messages.
As result, requestSignedCertFromFlipper fails, but it never triggers the change of protocol.
With this diff we start evaluating if we need to change our protocol for every socket error.
Reviewed By: lblasa
Differential Revision: D33890235
fbshipit-source-id: f79b5c6992f01f8a93e0793e180a5cbd4a105619
Summary:
Changelog: Initiate cert exchange when medium changes
Fixes the following bug:
1. Start an iOS app with WWW cert exchange
2. Get cert (and fake serial of a virtual device)
3. Close app
4. Restart Flipper
5. Switch app to FS_ACCESS cert exchange
6. Start app
Expected result:
App re-initializes cert exchange to get a new serial of a real device
Current result:
App tries to connect securely using the previous fake serial of a virtual device. Flipper does not recognize it and refuses the connection.
Reviewed By: lblasa
Differential Revision: D33163798
fbshipit-source-id: 67126a9b562f2cb7cfe6521a46abf38b2699fb2d
Summary:
This change makes WebSockets the default for Flipper on iOS.
Having said that, we are introducing some logic to deal with clients connecting to older Flipper Desktop versions.
The mobile client will first attempt to connect via WebSocket with the Desktop. This connection can either be secure or insecure. If that fails, it will attempt to connect via RSocket.
Connection failure logic:
The mobile client will attempt to connect up-to 3 times via a WebSocket. If it fails to connect, then the socket provider is switched to RSocket.
As before, the mobile client will attempt to connect up-to 3 times via a RSocket. If it fails to connect, then the socket provider is switched back to WebSocket.
Process repeats until a successful connection is established.
Some logs that can be seen from iOS:
2021-09-15 14:31:51.193503+0100 Sample[92026:92107440] [] nw_protocol_get_quic_image_block_invoke dlopen libquic failed
2021-09-15 14:31:51.878257+0100 Sample[92026:92107440] [connection] nw_socket_handle_socket_event [C1.1:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:52.553729+0100 Sample[92026:92107440] [connection] nw_socket_handle_socket_event [C1.2:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:52.899511+0100 Sample[92026:92107442] [connection] nw_connection_get_connected_socket [C1] Client called nw_connection_get_connected_socket on unconnected nw_connection
2021-09-15 14:31:52.899664+0100 Sample[92026:92107442] TCP Conn 0x600001d384d0 Failed : error 0:61 [61]
2021-09-15 14:31:57.120120+0100 Sample[92026:92107439] [connection] nw_socket_handle_socket_event [C2.1:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:57.141785+0100 Sample[92026:92107439] [connection] nw_socket_handle_socket_event [C2.2:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:57.151604+0100 Sample[92026:92107483] [connection] nw_connection_get_connected_socket [C2] Client called nw_connection_get_connected_socket on unconnected nw_connection
2021-09-15 14:31:57.154312+0100 Sample[92026:92107483] TCP Conn 0x600001d7c0b0 Failed : error 0:61 [61]
2021-09-15 14:31:59.206079+0100 Sample[92026:92107483] [connection] nw_socket_handle_socket_event [C3.1:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:59.236824+0100 Sample[92026:92107483] [connection] nw_socket_handle_socket_event [C3.2:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:31:59.251927+0100 Sample[92026:92107439] [connection] nw_connection_get_connected_socket [C3] Client called nw_connection_get_connected_socket on unconnected nw_connection
2021-09-15 14:31:59.255963+0100 Sample[92026:92107439] TCP Conn 0x600001d1c210 Failed : error 0:61 [61]
2021-09-15 14:32:01.291303+0100 Sample[92026:92107439] [connection] nw_socket_handle_socket_event [C4.1:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:32:01.312406+0100 Sample[92026:92107439] [connection] nw_socket_handle_socket_event [C4.2:1] Socket SO_ERROR [61: Connection refused]
2021-09-15 14:32:01.323099+0100 Sample[92026:92107483] [connection] nw_connection_get_connected_socket [C4] Client called nw_connection_get_connected_socket on unconnected nw_connection
2021-09-15 14:32:01.326028+0100 Sample[92026:92107483] TCP Conn 0x600001d7c0b0 Failed : error 0:61 [61]
flipper: Failed to connect with the current socket provider
flipper: Use legacy socket provider
flipper: FlipperClient::onConnected
Reviewed By: passy
Differential Revision: D30900471
fbshipit-source-id: 7c242ad71306803b050d0174fc22696bb74fdba5
Summary:
This change fixes a bug with the handled flag during the certificate exchange process.
Explanation:
handled was passed by reference as &handled
Once the function goes out of scope then the reference, well, it just becomes invalid (undefined behaviour)
In some cases, it appears as 'handled' because the reference is invalid and it happens to be 'true'.
Changelog: Fixed an issue where clients would randomly not connect to Flipper. Please update FlipperKit to 0.110.0 to apply the fix: https://fbflipper.com/docs/getting-started/react-native#using-the-latest-flipper-sdk
Reviewed By: mweststrate
Differential Revision: D31017592
fbshipit-source-id: c087a769fa23de1acfd3c198b4db4d6ccdb2be90
Summary:
Abstract the socket creation from FlipperConnectionManagerImpl. Instead, use FlipperSocketProvider.
There's a default provider which will always return RSocket sockets. This provider can be changed and thus can return other implementations.
Reviewed By: fabiomassimo
Differential Revision: D30396322
fbshipit-source-id: 0583865376809260b0240e5bd653d73f2fa514b1
Summary:
It's just bad that we give a naked pointer of the connection manager to other instances. If the connection manager gets deallocated, the instances keeping a pointer to it are doomed to crash.
This change creates a wrapper on top of the pointer that can be freely shared. On deallocation, the shared wrapper gets invalidated.
Reviewed By: timur-valiev
Differential Revision: D30398466
fbshipit-source-id: 8f228e7fbaebc0ea28921409de071b58bbb69f1e
Summary:
These changes abstract RSocket from FlipperConnectionManagerImpl.
This is achieved by defining FlipperSocket.
FlipperConnectionManagerImpl uses instances of FlipperSocket and RSocket is now contained within FlipperRSocket which implements FlipperSocket.
On the changes to follow, FlipperConnectionManagerImpl will no longer reference FlipperRSocket directly thus fully abstracting the socket implementation in use.
For reviewers:
- All of the RSocket code now lives in FlipperRSocket.
- Main changes are in FlipperConnectionManagerImpl. Lambdas are used to deal with events and message handling.
- There's some very minimal serialisation additions for payloads.
Reviewed By: jknoxville
Differential Revision: D30341076
fbshipit-source-id: 54bb4878967378490710c05f729cdd7f4cf08bb8
Summary:
RSocket plays nicely with Folly and OpenSSL.
Flipper WebSocket-client uses SocketRocket which instead relies on Apple's NSInputStream and NSOutputStream types.
SSL options can be set to secure the communication in both.
Unfortunately, Apple APIs are a bit limited on the supported cryptographic formats it can accept as arguments.
SSL options require the client certificate to be set in PKCS #12 format, contrary to the existing PEM format used by RSocket.
This change adds a method to the ConnectionContext which converts and saves the client certificate in PKCS #12 format.
The method is always expected to succeed as it will only be called once a valid client certificate is available. An unlikely failure will raise an exception.
Reviewed By: fabiomassimo
Differential Revision: D30074334
fbshipit-source-id: 91a475d080569cc339b649c7302b1f28793c7de7
Summary:
This diff adds upload and download logic for certs. It makes changes on both Flipper Client and Desktop side. With this we enable cert exchange through WWW.
Next Diffs:
1) Add Flipper state in cert provider for more debug data
2) Tests
Reviewed By: jknoxville
Differential Revision: D23092706
fbshipit-source-id: e576253606b64b62848b70203db7e09a3bd77fd9
Summary:
This diff adds a toggle setting in wilde which will enable certificate exchange through www.
Right now it just sends the information about which medium to be used for cert exchange to Flipper JS and its client side. But its implementation is not done yet.
### Flow for Wilde
Whenever user changes the setting(or when user logs out) we set the state of exchange medium and accordingly set/reset authtoken. Note at no given point we remove already existing certificates.
### Context for OSS
With this diff we introduce another way to do certificate exchange. Before this diff, we did certificate exchange by accessing the file system of app. But it turns out it's not possible to do that in applications signed by enterprise certs. Thus with this diff one can write their FlipperKitCertificateProvider and fetch the certificate from WWW.
Reviewed By: jknoxville
Differential Revision: D22896320
fbshipit-source-id: 55aef7028a62e71ba9c02f9f79acaab41d09c0c6
Summary:
Background for this diff: https://fb.quip.com/KqEfAlKYlgme
Some plugins don't respect that stuff (livefeed and graphql), but for others it seems to work fine.
This is just a PoC, there are some present bugs concerning the combination of selecting and bg plugins
Questions to investigate:
- [x] make sure that LiveFeed and GraphQL disconnect properly. There might be more plugins that need that
- [x] verifiy that we don't loose one of the original goals of background plugins, e.g. QPL collecting and sending data from device start. Does this still work as intended after this change?
- [x] how can we observe / measure improvements? Are dev builds more responsive after this? Is the layout inspector smoother for example because no QPL plugins are interweaved?
- [x] how is forward and backward compatibility?
- If Flipper is updated, but device not: No change I think, as getBackgroundPlugins() will return an empty set, and background plugins are initiated as usual, so old behavior
- If device is updated, but Flipper not, background plugins won't be started until they are selected. This is a degradation, but hopefully explainable.
- [x] Verify QPL buffer is not unbounded
- [x] Share architecutre changes with team
For Graphql updates: D20943455
Added runtime stats to monitor network traffic (sadly had to redo that since scuba couldn't handle the data format used at first, so probably will hold of landing this diff a week to make sure we can see some effects)
Follow up work:
[x] wait until we released the stat tracking before we release this, to be able to measure the effect?
[x] make sure graphql fix lands
[ ] use side effects abstraction
[ ] fix other background plugins (android only) or fix it in a generic way:
{F234394286}
Changelog: Background plugins will no longer receive a Flipper connection if they are disabled. This should significantly reduce the overall load of Flipper both on the device and desktop when unused plugins are disabled used, which could otherwise generate 10MB/s of network traffic certain scenarios. All plugins *should* be able to handle to this gracefully, but since this is quite a fundamental change, reach out to the Flipper team when in doubt!
Reviewed By: jknoxville
Differential Revision: D20942453
fbshipit-source-id: b699199cb95c1b3e4c36e026b6dfaee7d1652e1f
Summary:
Context: https://fb.workplace.com/groups/flippersupport/permalink/803808233433170/
A similar change was done earlier with the secure connection route.
This does the same for the insecure route.
Reviewed By: mweststrate
Differential Revision: D21227196
fbshipit-source-id: 844cb674b5b16033977f451bbc3d8bbc69732273
Summary:
Originally, Flipper would use exceptions for the control flow of determining if there's no desktop client available. It would catch an exception and then kick off another attempt. This is cool, but if you are debugging an application where Flipper is enabled, and you set a breakpoint to detect all thrown exceptions, then you'll see an exception thrown inside Flipper logic every couple of seconds. That makes it hard to debug what you're trying to do.
There's a workaround where you can simply open Flipper, but that's a bit annoying.
This diff changes the logic in Flipper to not use exceptions for the part of connection where it's doing the initial connect. We could apply this to `doCertificateExchange` as well, but that's actually an error there so I don't see the need to do it there as well.
Reviewed By: jknoxville
Differential Revision: D21016737
fbshipit-source-id: cbdfe2794b4d38a9e3b8304ebee845655bb26ae5
Summary:
The problem is that whenever an app is shutdown, and then reopened, the flipper dir gets reset when getting the CSR for connecting to flipper.
This causes the first connection attempt to fail always, and it goes through the whole cert exchange, taking longer than necessary.
Fixes it by loading the csr from disk if it's not loaded yet, without blowing away the whole certs state.
A side effect of this would be that as long as some file exists where the csr lives, flipper state would never get reset, so it wouldn't be able to fix itself automatically anymore. To keep that working, I've made `resetFlipperDir()` public and am calling it explicitly when starting certificate exchange. This should ensure that we still reset when we need to, but not unnecessarily.
The reason it went wrong is that getCSR used to be called only at cert exchange, when resetting and generating a new one was always desirable. However, when we shipped the fix for changeable android serials, it started to be used as a normal getter.
Reviewed By: timur-valiev
Differential Revision: D18834806
fbshipit-source-id: 56ca7e03e1aa9011f836bc9c021cf3048f7dc1e4
Summary:
There was an interesting problem in Flipper which I encountered. On closing our app we would deadlock. This was happening when joining the Flipper thread.
Upon investigation I found that it happened only when Flipper was not connected to the app. And digging in I found that it's because we are continually trying to connect the `FlipperClient` to the Flipper app when there is no connection. This works great, but it continues even if you have asked Flipper to terminate. So this meant that the `EventBase` never gets chance to shutdown, as it keeps getting new work to do.
The fix is to add a flag to `FlipperConnectionManagerImpl` to say if the conection is started or not. Then, if it's not started and we get a request to retry connection, then we will fail it.
Reviewed By: jknoxville
Differential Revision: D18780622
fbshipit-source-id: cce82cbb5c54e6d92ea16644c8a247bd2578ae26
Summary:
- Add CSR and CSR destination to request body
- (unrelated) Rearrange the list of include files
Reviewed By: jknoxville
Differential Revision: D17346429
fbshipit-source-id: ce775998884b73e82a931f4dd8986c659a892b51
Summary:
Don't fatal when trying to send a message to flipper that is too large.
Instead just continue to throw an exception, so the plugin will still be aware of the failure.
Reviewed By: cpojer, danielbuechele
Differential Revision: D17180334
fbshipit-source-id: 49c341a7cee579158be3878639083c6d19b3eaa3
Summary:
`/*` is the standard throughout open source code. For example, Firefox uses single /*: https://hg.mozilla.org/mozilla-central/file/21d22b2f541258d3d1cf96c7ba5ad73e96e616b5/gfx/ipc/CompositorWidgetVsyncObserver.cpp#l3
In addition, Rust considers `/**` to be a doc comment (similar to Javadoc) and having such a comment at the beginning of the file causes `rustc` to barf.
Note that some JavaScript tooling requires `/**`. This is OK since JavaScript files were not covered by the linter in the first place, but it would be good to have that tooling fixed too.
Reviewed By: zertosh
Differential Revision: D15640366
fbshipit-source-id: b4ed4599071516364d6109720750d6a43304c089
Summary:
Fixes T40544233
Exceptions should be thrown by value, not as a pointer, or else catching by reference (the recommended practice) won't work.
Reviewed By: priteshrnandgaonkar
Differential Revision: D14243337
fbshipit-source-id: 28677d7f1b3c025b67fb945f30116f3b751a89a8
Summary: Allows plugin to check if something is supported by that app, for example an extension command or a new feature, before trying to use it.
Reviewed By: passy
Differential Revision: D14225957
fbshipit-source-id: 3c5a29a06b56fe5f1da772824232547447872344
Summary: This info is still available in the diagnostic screen.
Reviewed By: passy
Differential Revision: D14164791
fbshipit-source-id: 1fc91da0b7437d530bddee6cbb5ea8c2d11f7b80
Summary: I got tripped up again with a physical iOS device that had the wrong date and time. SSL certs won't be valid. Adds a reminder to the diagnostic screen for anyone running into this in future.
Reviewed By: passy
Differential Revision: D14132411
fbshipit-source-id: 623120ef5252eae3e2fe0f6f653074f586f17e0e
Summary: The CSR has no need to change, don't recreate it every time we need it. Useful for the physical iOS case especially, because it connects to the portforwarder and keeps attempting to send a CSR.
Reviewed By: passy
Differential Revision: D14131617
fbshipit-source-id: 82a69d5aff813d77fa05dd13fc2441b784766d99
Summary:
We've seen some crashes from toRSocketPayload, but aren't sure of the cause.
When we fail the DCHECK, there won't be any log output, so log before it if it's going to fail.
Reviewed By: passy
Differential Revision: D14131409
fbshipit-source-id: b71e043f0b5feac3d2e44a34cc38c72086f966ad
Summary:
Every time an app connects to flipper, it will include its version.
With this information, the desktop app will be able to know how to communicate with it (or even whether it still can - and show an error message if not).
Reviewed By: passy
Differential Revision: D14008785
fbshipit-source-id: ac4dfd52da4bc6a2b90ec09a8e619086b7cd4a59
Summary:
Changing FlipperRSocketResponder to only use public parts of FlipperConnectionManagerImpl.
This means we can test it by injecting a FCM mock, and it can use its public interface.
Reviewed By: passy
Differential Revision: D14000078
fbshipit-source-id: c0431a888b0ca041807631c81b99fb8b947274d6
Summary:
For one thing, this file is too big so it's good to split it up.
Also, having this responder defined in the .cpp file makes it hard to test. Extracting it for testability.
Reviewed By: passy
Differential Revision: D14000079
fbshipit-source-id: 8da4e0e325f48c0ada8efc7cd6fffcb3440c6e26
Summary:
Flipper exposes a call() api to plugins which lets them call their sdk component, and it returns a promise with the response.
Currently this is done by sending a fireAndForget request, noting the id of the request, and then receiving fireAndForget requests and matching up the ids to give the result back to the right plugin promise.
Instead, it will be simpler to use rsocket requestResponse, instead of fireAndForget, which is for this exact use case. This diff adds a requestResponse handler to the SDK, so that it can deal with such requests and respond accordingly, while preserving the current functionality if it receives a fireAndForget.
So this part is backwards compatible and should be safe to land in isolation.
A later diff will change the desktop app to use requestResponse, which may not be backwards compatible, so that will have to be deployed more carefully.
Reviewed By: passy
Differential Revision: D13974049
fbshipit-source-id: b371d94a86b1f186375161ed8f2242a462ce418f
Summary:
Part 1 of enabling flipper to run on custom ports: android SDK.
Still to go: iOS SDK and desktop
This should allow you to run mobile apps that use flipper on different ports than the default (8089,8088).
`adb shell`
`su`
`setprop flipper.ports 1111,2222`
From what I can tell, this only works on rooted devices.
Reviewed By: passy
Differential Revision: D13753238
fbshipit-source-id: c5f370c9d8c7382e8c17fb81d4010c642ef7c114
Summary:
On iOS devices (internal only right now), when flipper desktop isn't running we get a Network Error instead of a Port not open error, because we can always connect to the port forwarding server running on the phone.
So in this case, don't count this error as a failure so we don't needlessly delete the certificates to attempt to fix it.
Reviewed By: passy
Differential Revision: D13358147
fbshipit-source-id: 8f9fb62cd2dcada058d104ef60f788a1b62d7094
Summary:
This does two things:
1. Change "Connect to desktop" to say which connection it's establishing instead, which makes it clearer what's actually happening. This step really does mean establishing a connection, because what happens during that connection is asynchronous after this step completes.
2. Call step->complete() after certificate exchange connection is established. This wasn't happening before, so you'd always get "[Failed] Connect to desktop" even after a successful cert exchange.
Reviewed By: priteshrnandgaonkar
Differential Revision: D13256783
fbshipit-source-id: 5e8e3a54f52d2e0adbde4c6d82d1acc840f1eb59
Summary: Wraps flipper client methods to avoid crash. Also added a tests which makes sure that malcious plugin cannot cause a crash
Reviewed By: jknoxville
Differential Revision: D13153277
fbshipit-source-id: ac21731fa3c4eb447f189e61f61b9e83aad91e13
Summary:
We're seeing some crashes when attempting to send payloads larger than are supported by rsocket.
Instead of crashing, skip the message, but log it so we can see what it's containing.
This is where the failure occurs: https://github.com/rsocket/rsocket-cpp/blob/master/rsocket/framing/FramedDuplexConnection.cpp#L64
Reviewed By: passy
Differential Revision: D12857616
fbshipit-source-id: 2b02d7f5dd6499ba81783d3f8aefcbb64d9d408a
Summary:
The code here is quite complicated so here's whats happening:
There's a disconnect hook, that waits and reconnects, so flipper never stops, except for the case where the desktop itself fails.
This makes some sense, but not total sense, as the client should really keep connecting, in case there's a different desktop that won't fail, or even if it was a transient failure.
Explicitly disconnecting, by setting the client to null, will trigger the disconnect hook and carry on the connect loop.
Reviewed By: passy
Differential Revision: D10231241
fbshipit-source-id: 90d823dce2221b1fb6501bd2797871dc1fb9770a
Summary:
This diff sets up flipper for running plugins in background. This diff does the following
- Adds a function named `runInBackground` to the interface `FlipperPlugin` to make the plugins opt in to be run in background, default is false
- Changes the javascript side of the flipper to store the messages received by the plugins in background
- Process the stored messages when the plugin in background becomes active
- Currently I have just turned on network plugin to be in background mode.
- Remove the buffering from the network plugin, as it will run in background
- Write a batching layer to batch the messages and send to flipper.
Note: I haven't tested the wilde app yet, but the sample app works. I will remove the "[WIP]" from the title once I have tested it in wilde
Reviewed By: danielbuechele
Differential Revision: D10301403
fbshipit-source-id: 034eebf659a545d6b480a4ac1b73b0aa4b2f9797
Summary: Part of sonar to flipper rename
Reviewed By: passy
Differential Revision: D9920275
fbshipit-source-id: 02f97d1e51d58864283d26e8d3a724cac973e938
Summary: Part of sonar to flipper rename
Reviewed By: passy
Differential Revision: D9919821
fbshipit-source-id: a44a2a04d5463750f884f8bf1328e02d56593e82
Summary:
Part of Sonar -> Flipper rename.
It's about time this is renamed from *Websocket as well, since it doesn't use websockets anymore.
Reviewed By: passy
Differential Revision: D9919695
fbshipit-source-id: 78a63bfb7d5de19c093b7fb775d1426b4fc58f77
