Issue 311253005: Bind to all available loopback addresses in pub.

Issue 311253005: Bind to all available loopback addresses in pub. (Closed)

Created:
6 years, 6 months ago by nweiz

Modified:
6 years, 6 months ago

Reviewers:
Bob Nystrom, Anders Johnsen

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Bind to all available loopback addresses in pub. R=rnystrom@google.com BUG=19147 Committed: https://code.google.com/p/dart/source/detail?r=37135

Patch Set 3 : code review #

Created: 6 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+95 lines, -60 lines)			Patch
M	pkg/scheduled_test/CHANGELOG.md	View	1	1 chunk	+5 lines, -0 lines	0 comments	Download
M	pkg/scheduled_test/lib/scheduled_server.dart	View	1	3 chunks	+7 lines, -5 lines	0 comments	Download
M	pkg/scheduled_test/pubspec.yaml	View	1	2 chunks	+2 lines, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/lib/src/barback/admin_server.dart	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/lib/src/barback/asset_environment.dart	View	1	1 chunk	+5 lines, -4 lines	0 comments	Download
M	sdk/lib/_internal/pub/lib/src/barback/barback_server.dart	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/lib/src/command/build.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/lib/src/io.dart	View		2 chunks	+10 lines, -0 lines	0 comments	Download
M	sdk/lib/_internal/pub/lib/src/oauth2.dart	View	1	1 chunk	+16 lines, -17 lines	0 comments	Download
M	sdk/lib/_internal/pub/lib/src/source/hosted.dart	View	1	2 chunks	+4 lines, -12 lines	0 comments	Download
M	sdk/lib/_internal/pub/lib/src/utils.dart	View	1	2 chunks	+25 lines, -0 lines	0 comments	Download
M	sdk/lib/_internal/pub/test/cache/repair/handles_failure_test.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/test/cache/repair/reinstalls_hosted_packages_test.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/test/descriptor.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/test/get/hosted/get_test.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/test/hosted/fail_gracefully_on_missing_package_test.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/test/hosted/version_negotiation_test.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/test/list_package_dirs/lists_dependency_directories_test.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/test/serve/utils.dart	View	1 2	5 chunks	+5 lines, -6 lines	0 comments	Download
M	sdk/lib/_internal/pub/test/serve/web_socket/serve_directory_and_immediately_unserve_test.dart	View	1	1 chunk	+1 line, -2 lines	0 comments	Download
M	sdk/lib/_internal/pub/test/serve/web_socket/serve_directory_test.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/_internal/pub/test/test_pub.dart	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

Bob Nystrom

Can you also change any code we have that's explicitly using 127.0.0.1 to use localhost ...

6 years, 6 months ago (2014-06-05 16:22:19 UTC) #2

nweiz

> Can you also change any code we have that's explicitly using 127.0.0.1 to use ...

6 years, 6 months ago (2014-06-05 21:36:20 UTC) #3

Bob Nystrom

LGTM. https://codereview.chromium.org/311253005/diff/20001/sdk/lib/_internal/pub/test/serve/utils.dart File sdk/lib/_internal/pub/test/serve/utils.dart (right): https://codereview.chromium.org/311253005/diff/20001/sdk/lib/_internal/pub/test/serve/utils.dart#newcode147 sdk/lib/_internal/pub/test/serve/utils.dart:147: "--hostname=localhost", // Force IPv4 on bots. This can ...

6 years, 6 months ago (2014-06-06 20:22:04 UTC) #4

nweiz

https://codereview.chromium.org/311253005/diff/20001/sdk/lib/_internal/pub/test/serve/utils.dart File sdk/lib/_internal/pub/test/serve/utils.dart (right): https://codereview.chromium.org/311253005/diff/20001/sdk/lib/_internal/pub/test/serve/utils.dart#newcode147 sdk/lib/_internal/pub/test/serve/utils.dart:147: "--hostname=localhost", // Force IPv4 on bots. On 2014/06/06 20:22:03, ...

6 years, 6 months ago (2014-06-09 20:01:19 UTC) #5

nweiz

Committed patchset #3 manually as r37135 (presubmit successful).

6 years, 6 months ago (2014-06-09 20:04:04 UTC) #6

Anders Johnsen

DBC: My understanding is that it's more likely that a proxy will capture 'localhost' than ...

6 years, 6 months ago (2014-06-09 20:09:50 UTC) #7

nweiz

6 years, 6 months ago (2014-06-09 21:17:09 UTC) #8

Message was sent while issue was closed.

On 2014/06/09 20:09:50, Anders Johnsen wrote:
> DBC:
> 
> My understanding is that it's more likely that a proxy will capture
'localhost'
> than the raw address. Perhaps it would it would be safest to keep the
127.0.0.1,
> but still listen to both IPv4 and IPv6. That way we can be as conservative as
> possible.

We'd really like to print "localhost" on the command line by default, since it's
much more user-friendly than a sequence of numbers. However, this change means
we're never binding to localhost internally, so users should be able to manually
enter "127.0.0.1" or "[::1]" in their URL bar to avoid some proxy issues.

Expand Messages | Collapse Messages