Allow viaduct to make requests to the android emulator loopback address. #5270
Conversation
mhammond
force-pushed
the
viaduct-android-localhost
branch
2 times, most recently
from
d787c3a to
6d3c24f
Compare
|
I can't formally ask @badboy for review, so consider this an informal request :) |
Codecov ReportBase: 44.30% // Head: 44.31% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #5270 +/- ##
=======================================
Coverage 44.30% 44.31%
=======================================
Files 175 175
Lines 13898 13929 +31
=======================================
+ Hits 6158 6173 +15
- Misses 7740 7756 +16
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
CHANGES_UNRELEASED.md
Outdated
| ## Viaduct | ||
|
|
||
| ### What's New | ||
| - Allow viaduct to make requests to the android emulator loopback address via | ||
| a new viaduct_allow_android_emulator_loopback() (in Rust)/allowAndroidEmulatorLoopback() (in Kotlin) | ||
| ([#5270](https://github.com/mozilla/application-services/pull/5270)) | ||
|
|
There was a problem hiding this comment.
This should be moved below line 20 (outside of the <!-- ... --> comment block)
components/viaduct/Cargo.toml
Outdated
| @@ -11,10 +11,12 @@ crate-type = ["lib"] | |||
|
|
|||
| [dependencies] | |||
| url = "2.1" # mozilla-central can't yet take 2.2 (see bug 1734538) | |||
| lazy_static = "1.4" | |||
There was a problem hiding this comment.
viaduct now uses both lazy_static and once_cell.
As a followup this should be trimmed down to only one (probably once_cell as the more "modern" variant and the API that's gonna get into libstd eventually)
There was a problem hiding this comment.
Ah, this is of course the first usage of lazy_static here. Maybe just use once_cell then? We can migrate lazy_static in viaduct-reqwest later.
| @@ -54,6 +54,17 @@ object RustHttpConfig { | |||
| } | |||
| } | |||
|
|
|||
| /** Allows connections to the hard-coded address the Android Emulator uses for | |||
| * localhost (ie, http://10.0.2.2) - if you don't call this, viaduct will fail to | |||
There was a problem hiding this comment.
| * localhost (ie, http://10.0.2.2) - if you don't call this, viaduct will fail to | |
| * the host's localhost (ie, http://10.0.2.2) - if you don't call this, viaduct will fail to |
or something like that? Or leave out localhost and call it the host machine?
mhammond
force-pushed
the
viaduct-android-localhost
branch
from
6d3c24f to
0a9d36b
Compare
Viaduct itself gets the ability to allow exactly 1 arbitrary address to be connected to which isn't a https:// URL, but the FFI adds an android specific function which hard-codes the accepted address.
mhammond
force-pushed
the
viaduct-android-localhost
branch
from
0a9d36b to
b6a0e53
Compare
|
Thanks! Applied all suggestions and even made viaduct-request use once_cell instead of lazy_static! |
Viaduct itself gets the ability to allow exactly 1 arbitrary address to be connected to which isn't a https:// URL, but the FFI adds an android specific function which hard-codes the accepted address.
@csadilek said this seems to work in his testing. I don't think it's a breaking change but will require an a-c patch to expose the new method.
@badboy already pointed out my check should check just the host, but I also included the scheme. LMK if you would prefer different semantics.
Pull Request checklist
[ci full]to the PR title.Branch builds: add
[ff-android: firefox-android-branch-name]and/or[fenix: fenix-branch-name]to the PR title.