Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add support for setting allow http flag in Dart VM #17653

Merged
merged 4 commits into from
Apr 11, 2020
Merged

Add support for setting allow http flag in Dart VM #17653

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9c38743
Add support for setting allow http flag in Dart VM
mehmetf Apr 11, 2020
2ce103f
Fix imports
mehmetf Apr 11, 2020
f2a0f47
Fix build deps too
mehmetf Apr 11, 2020
aed53b2
Make formatter happy
mehmetf Apr 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions common/settings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ struct Settings {
bool endless_trace_buffer = false;
bool enable_dart_profiling = false;
bool disable_dart_asserts = false;

// Used to signal the embedder whether HTTP connections are disabled.
bool disable_http = false;

// Used as the script URI in debug messages. Does not affect how the Dart code
// is executed.
std::string advisory_script_uri = "main.dart";
Expand Down
1 change: 1 addition & 0 deletions lib/io/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ source_set("io") {
]

deps = [
"//flutter/fml",
"//flutter/third_party/tonic",
"//third_party/dart/runtime:dart_api",
"//third_party/dart/runtime/bin:dart_io_api",
Expand Down
18 changes: 14 additions & 4 deletions lib/io/dart_io.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,31 @@

#include "flutter/lib/io/dart_io.h"

#include "flutter/fml/logging.h"

#include "third_party/dart/runtime/include/bin/dart_io_api.h"
#include "third_party/dart/runtime/include/dart_api.h"
#include "third_party/tonic/converter/dart_converter.h"
#include "third_party/tonic/logging/dart_error.h"

using tonic::LogIfError;
using tonic::ToDart;

namespace flutter {

void DartIO::InitForIsolate() {
void DartIO::InitForIsolate(bool disable_http) {
Dart_Handle result = Dart_SetNativeResolver(
Dart_LookupLibrary(ToDart("dart:io")), dart::bin::LookupIONative,
dart::bin::LookupIONativeSymbol);
if (Dart_IsError(result)) {
Dart_PropagateError(result);
}
FML_CHECK(!LogIfError(result));

// The SDK expects this field to represent "allow http" so we switch the
// value.
Dart_Handle allow_http_value = disable_http ? Dart_False() : Dart_True();
Comment on lines +25 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the SDK uses "allow", why use "disable" here? The inconsistency (especially in C++ where there are not named parameters) increases the likelihood for a potential mismatch.

(The style guide for the flutter/flutter repo discourages negative names. Granted, that's not a style guide for flutter/engine, but I think it's still a good convention to follow.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I struggled with this too. The problem is that boolean switches in engine currently don't test for values. They simply infer true if the switch exists and false if it does not. This works well only for settings whose default value is false.

I would ideally have a switch for allow_http and then be able to do --noallow_http from the embedder. My understanding is that that would require a large-ish change to the switch processing (and it is only relevant for Android). I figured it is not worth doing especially with many examples of negative names in engine already.

Another simple way to solve this is to have an explicit switch for --noallow_http. But that doesn't really solve the problem. You still end up with a negative name and the negation is pushed to somewhere else.

Dart_Handle set_field_result =
Dart_SetField(Dart_LookupLibrary(ToDart("dart:_http")),
ToDart("_embedderAllowsHttp"), allow_http_value);
FML_CHECK(!LogIfError(set_field_result));
}

} // namespace flutter
2 changes: 1 addition & 1 deletion lib/io/dart_io.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace flutter {

class DartIO {
public:
static void InitForIsolate();
static void InitForIsolate(bool disable_http);

private:
FML_DISALLOW_IMPLICIT_CONSTRUCTORS(DartIO);
Expand Down
5 changes: 3 additions & 2 deletions runtime/dart_isolate.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ DartIsolate::DartIsolate(const Settings& settings,
settings.log_tag,
settings.unhandled_exception_callback,
DartVMRef::GetIsolateNameServer()),
is_root_isolate_(is_root_isolate) {
is_root_isolate_(is_root_isolate),
disable_http_(settings.disable_http) {
phase_ = Phase::Uninitialized;
}

Expand Down Expand Up @@ -261,7 +262,7 @@ bool DartIsolate::LoadLibraries() {

tonic::DartState::Scope scope(this);

DartIO::InitForIsolate();
DartIO::InitForIsolate(disable_http_);

DartUI::InitForIsolate(IsRootIsolate());

Expand Down
1 change: 1 addition & 0 deletions runtime/dart_isolate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ class DartIsolate : public UIDartState {
std::vector<std::unique_ptr<AutoFireClosure>> shutdown_callbacks_;
fml::RefPtr<fml::TaskRunner> message_handling_task_runner_;
const bool is_root_isolate_;
const bool disable_http_;

DartIsolate(const Settings& settings,
TaskRunners task_runners,
Expand Down
3 changes: 3 additions & 0 deletions shell/common/switches.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,9 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) {
}
}

settings.disable_http =
command_line.HasOption(FlagForSwitch(Switch::DisableHttp));

// Disable need for authentication codes for VM service communication, if
// specified.
settings.disable_service_auth_codes =
Expand Down
6 changes: 6 additions & 0 deletions shell/common/switches.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,12 @@ DEF_SWITCH(DisableDartAsserts,
"disabled. This flag may be specified if the user wishes to run "
"with assertions disabled in the debug product mode (i.e. with JIT "
"or DBC).")
DEF_SWITCH(DisableHttp,
"disable-http",
"Dart VM has a master switch that can be set to disable insecure "
"HTTP and WebSocket protocols. Localhost or loopback addresses are "
"exempted. This flag can be specified if the embedder wants this "
"for a particular platform.")
DEF_SWITCH(
ForceMultithreading,
"force-multithreading",
Expand Down