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
