Commit Graph

32 Commits

Author SHA1 Message Date
Pritesh Nandgaonkar
293de19c2b Add toggle in the settings for cert exchange medium
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
2020-08-12 04:44:18 -07:00
Alex Dickson
0065ddedd7 Provide explicit failure reason when connection can't be established
Reviewed By: jknoxville

Differential Revision: D22999785

fbshipit-source-id: 301ec8230a89ee9dbc82d006fa6520d8bac80b3e
2020-08-10 04:31:12 -07:00
Michel Weststrate
b9c3d99f44 Stop connecting disabled background plugins
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
2020-04-27 09:46:13 -07:00
John Knox
eb1981f9f2 Don't log when insecure connection fails
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
2020-04-24 05:52:14 -07:00
Matt Galloway
a1919bc4ef Flipper | Stop exceptions being thrown for indicating that no Flipper desktop client is available
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
2020-04-16 06:47:40 -07:00
John Knox
c2370f7faf Speed up re-connect after re-opening app
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
2019-12-06 05:29:22 -08:00
Matt Galloway
d37c64c329 Flipper | Fix potential for deadlock when stopping Flipper
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
2019-12-04 07:33:10 -08:00
Andres Suarez
0675dd924d Tidy up Flipper license headers [1/2]
Reviewed By: passy

Differential Revision: D17863711

fbshipit-source-id: 259dc77826fb803ff1b88c88529d7f679d3b74d8
2019-10-11 13:46:45 -07:00
Chaiwat Ekkaewnumchai
88197a7076 Fix Android Connection Swap Issue (Client Side)
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
2019-09-16 05:33:19 -07:00
John Knox
6eb1531aae Remove rsocket payload size assertion
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
2019-09-04 04:04:22 -07:00
Rain ⁣
aa649ff48f standardize C-like MIT copyright headers throughout fbsource
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
2019-06-06 19:40:28 -07:00
John Knox
56576a84ce Fix app crash when payload is too large
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
2019-02-27 10:47:04 -08:00
John Knox
40ada838d7 Add isMethodSupported handler
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
2019-02-26 10:08:21 -08:00
John Knox
5fff501d26 Don't log cert exchange connection failures
Summary: This info is still available in the diagnostic screen.

Reviewed By: passy

Differential Revision: D14164791

fbshipit-source-id: 1fc91da0b7437d530bddee6cbb5ea8c2d11f7b80
2019-02-21 10:12:04 -08:00
John Knox
81e8b7982b Add date and time reminder to diagnostic screen
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
2019-02-19 10:33:25 -08:00
John Knox
9c87dfe230 Only create CSR once per app invocation
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
2019-02-19 10:33:25 -08:00
John Knox
6cd8e1519f Log start of payloads that are too big to send
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
2019-02-19 08:50:30 -08:00
John Knox
0413bbd458 Add sdk_version param when connecting to flipper
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
2019-02-11 14:06:56 -08:00
John Knox
5da8f35ee3 Don't use private access in FlipperRSocketResponder
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
2019-02-11 14:06:56 -08:00
John Knox
c48c1a728a Extract Responder class out of FlipperConnectionManagerImpl
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
2019-02-11 14:06:56 -08:00
John Knox
4a3de26a88 Add requestResponse handler for incoming calls
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
2019-02-11 14:06:55 -08:00
John Knox
324a7ae873 Enable flipper android to use different ports based on prop
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
2019-01-24 03:12:13 -08:00
John Knox
dc313a460c Don't invalidate connection files on Network Errors
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
2018-12-06 06:53:22 -08:00
John Knox
28e2190635 Improve "Connect to desktop" diagnostic step
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
2018-11-30 07:04:10 -08:00
Pritesh Nandgaonkar
bf3be3e16c Wrap flipper client methods to avoid crash
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
2018-11-26 07:56:13 -08:00
John Knox
db833d36e9 Add max rsocket payload size
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
2018-10-31 11:14:56 -07:00
John Knox
57752533a6 Reconnect after desktop fails during cert exchange
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
2018-10-24 10:37:16 -07:00
Pritesh Nandgaonkar
5bbfa58909 Setup sdk for background plugin
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
2018-10-11 15:23:21 -07:00
John Knox
1a076f1300 Rename C++ fields and internal methods
Summary: From sonar* to flipper*.

Reviewed By: priteshrnandgaonkar

Differential Revision: D9944461

fbshipit-source-id: 870997e3d1d5aaef73dd445e6d94615f93fe6abc
2018-09-25 04:57:48 -07:00
John Knox
8ae77810d9 Rename SonarStep
Summary: Part of sonar to flipper rename

Reviewed By: passy

Differential Revision: D9920275

fbshipit-source-id: 02f97d1e51d58864283d26e8d3a724cac973e938
2018-09-24 06:03:51 -07:00
John Knox
c1295b1bc9 Rename SonarState* to FlipperState*
Summary: Part of sonar to flipper rename

Reviewed By: passy

Differential Revision: D9919821

fbshipit-source-id: a44a2a04d5463750f884f8bf1328e02d56593e82
2018-09-24 06:03:51 -07:00
John Knox
1d2793f701 Rename SonarWebSocket
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
2018-09-24 06:03:51 -07:00