Add command line flag to allow testing clients not connected to localhost. Otherwise, restrict conn…

Add command line flag to allow testing clients not connected to localhost. Otherwise, restrict connections to localhost

BUG=582102
TEST=Run the engine without the "listen-outside" flag and make sure incoming connections are limited to localhost. Run with --listen-outside=1 and make sure it allows incomiong connections from outside localhost

Committed: https://crrev.com/07d76104b834de84788d97ec4c46fd723eb2196d
Cr-Commit-Position: refs/heads/master@{#415687}

[steimel, 2016-08-30 00:02:07]

Description was changed from

==========
Add command line flag to allow testing clients not connected to localhost.
Otherwise, restrict connections to localhost

BUG=582102
TEST=Run the engine without the "listen-outside" flag and make sure incoming
connections are limited to localhost. Run with --listen-outside=1 and make sure
it allows incomiong connections from outside localhost
==========

to

==========
Add command line flag to allow testing clients not connected to localhost.
Otherwise, restrict connections to localhost

BUG=582102
TEST=Run the engine without the "listen-outside" flag and make sure incoming
connections are limited to localhost. Run with --listen-outside=1 and make sure
it allows incomiong connections from outside localhost
==========

[steimel, 2016-08-30 00:02:07]

steimel@chromium.org changed reviewers:
+ wez@chromium.org

[steimel, 2016-08-30 20:25:15]

ptal

[Wez, 2016-08-30 21:42:01]

LGTM w/ some (mainly naming) nits. :)

https://codereview.chromium.org/2292563003/diff/1/blimp/engine/app/switches.cc
File blimp/engine/app/switches.cc (right):

https://codereview.chromium.org/2292563003/diff/1/blimp/engine/app/switches.c...
blimp/engine/app/switches.cc:11: const char kEngineListenOutside[] =
"listen-outside";
Suggest renaming this to something like "allow-non-localhost".

https://codereview.chromium.org/2292563003/diff/1/blimp/engine/app/switches.h
File blimp/engine/app/switches.h (right):

https://codereview.chromium.org/2292563003/diff/1/blimp/engine/app/switches.h...
blimp/engine/app/switches.h:14: // Boolean indicating whether or not to listen
outside of localhost.
nit: Suggest something like "Flag enabling access by clients from non-localhost
IP addresses."

https://codereview.chromium.org/2292563003/diff/1/blimp/engine/session/blimp_...
File blimp/engine/session/blimp_engine_session.cc (right):

https://codereview.chromium.org/2292563003/diff/1/blimp/engine/session/blimp_...
blimp/engine/session/blimp_engine_session.cc:103: net::IPAddress
GetListeningIPAddress() {
nit: I'd suggest just GetListeningAddress, since IP is more-or-less implicit at
this level :)

https://codereview.chromium.org/2292563003/diff/1/blimp/engine/session/blimp_...
blimp/engine/session/blimp_engine_session.cc:105: kEngineListenOutside) == "1")
{
Since this is a Boolean parameter and defaults to false, you can just use
HasSwitch() to determine if the switch is present here, then the app gets run
with --my-option rather than --my-option=1

[steimel, 2016-08-31 17:33:05]

The CQ bit was checked by steimel@chromium.org

[steimel, 2016-08-31 17:33:05]

The patchset sent to the CQ was uploaded after l-g-t-m from wez@chromium.org
Link to the patchset: https://codereview.chromium.org/2292563003/#ps20001
(title: "Address review comments")

[commit-bot: I haz the power, 2016-08-31 17:33:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 18:29:49]

Description was changed from

==========
Add command line flag to allow testing clients not connected to localhost.
Otherwise, restrict connections to localhost

BUG=582102
TEST=Run the engine without the "listen-outside" flag and make sure incoming
connections are limited to localhost. Run with --listen-outside=1 and make sure
it allows incomiong connections from outside localhost
==========

to

==========
Add command line flag to allow testing clients not connected to localhost.
Otherwise, restrict connections to localhost

BUG=582102
TEST=Run the engine without the "listen-outside" flag and make sure incoming
connections are limited to localhost. Run with --listen-outside=1 and make sure
it allows incomiong connections from outside localhost
==========

[commit-bot: I haz the power, 2016-08-31 18:29:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-31 18:32:38]

Description was changed from

==========
Add command line flag to allow testing clients not connected to localhost.
Otherwise, restrict connections to localhost

BUG=582102
TEST=Run the engine without the "listen-outside" flag and make sure incoming
connections are limited to localhost. Run with --listen-outside=1 and make sure
it allows incomiong connections from outside localhost
==========

to

==========
Add command line flag to allow testing clients not connected to localhost.
Otherwise, restrict connections to localhost

BUG=582102
TEST=Run the engine without the "listen-outside" flag and make sure incoming
connections are limited to localhost. Run with --listen-outside=1 and make sure
it allows incomiong connections from outside localhost

Committed: https://crrev.com/07d76104b834de84788d97ec4c46fd723eb2196d
Cr-Commit-Position: refs/heads/master@{#415687}
==========

[commit-bot: I haz the power, 2016-08-31 18:32:40]

Patchset 2 (id:??) landed as
https://crrev.com/07d76104b834de84788d97ec4c46fd723eb2196d
Cr-Commit-Position: refs/heads/master@{#415687}