Skip to content

Commit 222b894

Browse files
lrhncommit-bot@chromium.org
lrhn
authored and
[email protected]
committed
Change resolve for package:URIs to not remove package name.
A "package URI" is defined as one with a `package` scheme, no authority, a first path segment terminated by `/` which contains no escapes and is not all `.` characters. This is the definition of package names accepted by .packages as well: Valid path characters and not all dots. Change-Id: I9a161d47732e8bf873d278774315c72a4a928823 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/117542 Commit-Queue: Lasse R.H. Nielsen <[email protected]> Reviewed-by: Nate Bosch <[email protected]>
1 parent 5bfeed2 commit 222b894

File tree

4 files changed

+286
-4
lines changed

4 files changed

+286
-4
lines changed

‎pkg/analysis_server/test/src/services/correction/assist/convert_to_package_import_test.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ import 'package:test/foo/bar.dart';
8585
''');
8686
}
8787

88+
@FailingTest(issue: 'http://dartbug.com/44871')
8889
Future<void> test_relativeImport_noAssistWithLint() async {
8990
createAnalysisOptionsFile(lints: [LintNames.avoid_relative_lib_imports]);
9091
verifyNoTestUnitErrors = false;

‎sdk/lib/core/uri.dart

Lines changed: 85 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2486,6 +2486,23 @@ class _Uri implements Uri {
24862486
return resolveUri(Uri.parse(reference));
24872487
}
24882488

2489+
// Returns the index of the `/` after the package name of a package URI.
2490+
//
2491+
// Returns negative if the URI is not a valid package URI:
2492+
// * Scheme must be "package".
2493+
// * No authority.
2494+
// * Path starts with "something"/
2495+
// * where "something" is not all "." characters,
2496+
// * and contains no escapes or colons.
2497+
//
2498+
// The characters are necessarily valid path characters.
2499+
static int _packageNameEnd(Uri uri, String path) {
2500+
if (uri.isScheme("package") && !uri.hasAuthority) {
2501+
return _skipPackageNameChars(path, 0, path.length);
2502+
}
2503+
return -1;
2504+
}
2505+
24892506
Uri resolveUri(Uri reference) {
24902507
// From RFC 3986.
24912508
String targetScheme;
@@ -2526,7 +2543,22 @@ class _Uri implements Uri {
25262543
targetQuery = this._query;
25272544
}
25282545
} else {
2529-
if (reference.hasAbsolutePath) {
2546+
String basePath = this.path;
2547+
int packageNameEnd = _packageNameEnd(this, basePath);
2548+
if (packageNameEnd > 0) {
2549+
assert(targetScheme == "package");
2550+
assert(!this.hasAuthority);
2551+
assert(!this.hasEmptyPath);
2552+
// Merging a path into a package URI.
2553+
String packageName = basePath.substring(0, packageNameEnd);
2554+
if (reference.hasAbsolutePath) {
2555+
targetPath = packageName + _removeDotSegments(reference.path);
2556+
} else {
2557+
targetPath = packageName +
2558+
_removeDotSegments(_mergePaths(
2559+
basePath.substring(packageName.length), reference.path));
2560+
}
2561+
} else if (reference.hasAbsolutePath) {
25302562
targetPath = _removeDotSegments(reference.path);
25312563
} else {
25322564
// This is the RFC 3986 behavior for merging.
@@ -4278,6 +4310,25 @@ class _SimpleUri implements Uri {
42784310
return _toNonSimple().resolveUri(reference);
42794311
}
42804312

4313+
// Returns the index of the `/` after the package name of a package URI.
4314+
//
4315+
// Returns negative if the URI is not a valid package URI:
4316+
// * Scheme must be "package".
4317+
// * No authority.
4318+
// * Path starts with "something"/
4319+
// * where "something" is not all "." characters,
4320+
// * and contains no escapes or colons.
4321+
//
4322+
// The characters are necessarily valid path characters.
4323+
static int _packageNameEnd(_SimpleUri uri) {
4324+
if (uri._isPackage && !uri.hasAuthority) {
4325+
// Becomes Non zero if seeing any non-dot character.
4326+
// Also guards against empty package names.
4327+
return _skipPackageNameChars(uri._uri, uri._pathStart, uri._queryStart);
4328+
}
4329+
return -1;
4330+
}
4331+
42814332
// Merge two simple URIs. This should always result in a prefix of
42824333
// one concatenated with a suffix of the other, possibly with a `/` in
42834334
// the middle of two merged paths, which is again simple.
@@ -4345,8 +4396,11 @@ class _SimpleUri implements Uri {
43454396
return base.removeFragment();
43464397
}
43474398
if (ref.hasAbsolutePath) {
4348-
var delta = base._pathStart - ref._pathStart;
4349-
var newUri = base._uri.substring(0, base._pathStart) +
4399+
int basePathStart = base._pathStart;
4400+
int packageNameEnd = _packageNameEnd(this);
4401+
if (packageNameEnd > 0) basePathStart = packageNameEnd;
4402+
var delta = basePathStart - ref._pathStart;
4403+
var newUri = base._uri.substring(0, basePathStart) +
43504404
ref._uri.substring(ref._pathStart);
43514405
return _SimpleUri(
43524406
newUri,
@@ -4393,7 +4447,12 @@ class _SimpleUri implements Uri {
43934447
String refUri = ref._uri;
43944448
int baseStart = base._pathStart;
43954449
int baseEnd = base._queryStart;
4396-
while (baseUri.startsWith("../", baseStart)) baseStart += 3;
4450+
int packageNameEnd = _packageNameEnd(this);
4451+
if (packageNameEnd >= 0) {
4452+
baseStart = packageNameEnd; // At the `/` after the first package name.
4453+
} else {
4454+
while (baseUri.startsWith("../", baseStart)) baseStart += 3;
4455+
}
43974456
int refStart = ref._pathStart;
43984457
int refEnd = ref._queryStart;
43994458

@@ -4544,3 +4603,25 @@ int _stringOrNullLength(String? s) => (s == null) ? 0 : s.length;
45444603

45454604
List<String> _toUnmodifiableStringList(String key, List<String> list) =>
45464605
List<String>.unmodifiable(list);
4606+
4607+
/// Counts valid package name characters in [source].
4608+
///
4609+
/// If [source] starts at [start] with a valid package name,
4610+
/// followed by a `/`, no later than [end],
4611+
/// then the position of the `/` is returned.
4612+
/// If not, a negative value is returned.
4613+
/// (Assumes source characters are valid path characters.)
4614+
/// A name only consisting of `.` characters is not a valid
4615+
/// package name.
4616+
int _skipPackageNameChars(String source, int start, int end) {
4617+
// Becomes non-zero when seeing a non-dot character.
4618+
// Also guards against empty package names.
4619+
var dots = 0;
4620+
for (var i = start; i < end; i++) {
4621+
var char = source.codeUnitAt(i);
4622+
if (char == _SLASH) return (dots != 0) ? i : -1;
4623+
if (char == _PERCENT || char == _COLON) return -1;
4624+
dots |= char ^ _DOT;
4625+
}
4626+
return -1;
4627+
}

‎tests/corelib/uri_test.dart

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,105 @@ void testReplace() {
474474
Expect.listEquals(["43", "38"], params["y"]!);
475475
}
476476

477+
void testPackageUris() {
478+
// A URI is recognized as a package URI if it has:
479+
// * "package" as scheme
480+
// * no authority
481+
// * a first path segment
482+
// * containing no `%`,
483+
// * which is not all "." characters,
484+
// * and which ends with a `/`.
485+
//
486+
// If so, the package name is unaffected by path resolution.
487+
var uri = Uri.parse("package:foo/bar/baz"); // Simple base URI.
488+
489+
Expect.stringEquals("package:foo/qux", // Resolve simple URI.
490+
uri.resolve("../../qux").toString());
491+
492+
Expect.stringEquals("package:foo/qux",
493+
uri.resolve("/qux").toString());
494+
495+
Expect.stringEquals("package:foo/qux?%2F", // Resolve non-simple URI.
496+
uri.resolve("../../qux?%2F").toString());
497+
498+
Expect.stringEquals("package:foo/qux?%2F",
499+
uri.resolve("/qux?%2F").toString());
500+
501+
uri = Uri.parse("package:foo/%62ar/baz"); // Non-simple base URI.
502+
503+
Expect.stringEquals("package:foo/qux", // Resolve simple URI.
504+
uri.resolve("../../qux").toString());
505+
506+
Expect.stringEquals("package:foo/qux",
507+
uri.resolve("/qux").toString());
508+
509+
Expect.stringEquals("package:foo/qux?%2F", // Resolve non-simple URI.
510+
uri.resolve("../../qux?%2F").toString());
511+
512+
Expect.stringEquals("package:foo/qux?%2F",
513+
uri.resolve("/qux?%2F").toString());
514+
515+
// The following base URIs are not recognized as package URIs:
516+
uri = Uri.parse("puckage:foo/bar/baz"); // Not "package" scheme.
517+
518+
Expect.stringEquals("puckage:/qux",
519+
uri.resolve("../../qux").toString());
520+
521+
Expect.stringEquals("puckage:/qux",
522+
uri.resolve("/qux").toString());
523+
524+
uri = Uri.parse("package://foo/bar/baz"); // Has authority.
525+
526+
Expect.stringEquals("package://foo/qux",
527+
uri.resolve("../../qux").toString());
528+
529+
Expect.stringEquals("package://foo/qux",
530+
uri.resolve("/qux").toString());
531+
532+
uri = Uri.parse("package:/foo/bar/baz"); // Has empty package name.
533+
534+
Expect.stringEquals("package:/qux",
535+
uri.resolve("../../qux").toString());
536+
537+
Expect.stringEquals("package:/qux",
538+
uri.resolve("/qux").toString());
539+
540+
uri = Uri.parse("package:f%2fo/bar/baz"); // Has escape in package name.
541+
542+
Expect.stringEquals("package:/qux",
543+
uri.resolve("../../qux").toString());
544+
545+
Expect.stringEquals("package:/qux",
546+
uri.resolve("/qux").toString());
547+
548+
uri = Uri.parse("package:f:o/bar/baz"); // Has colon in package name.
549+
550+
Expect.stringEquals("package:/qux",
551+
uri.resolve("../../qux").toString());
552+
553+
Expect.stringEquals("package:/qux",
554+
uri.resolve("/qux").toString());
555+
556+
uri = Uri.parse("package:.../bar/baz"); // Has only '.' in package name.
557+
558+
Expect.stringEquals("package:/qux",
559+
uri.resolve("../../qux").toString());
560+
561+
Expect.stringEquals("package:/qux",
562+
uri.resolve("/qux").toString());
563+
564+
uri = Uri.parse("package:foo?/"); // Has no `/` after package name.
565+
566+
// Resolving relative against non-absolute path gives
567+
// a non-absolute path again.
568+
// TODO(lrn): Is this a bug?
569+
Expect.stringEquals("package:qux",
570+
uri.resolve("../../qux").toString());
571+
572+
Expect.stringEquals("package:/qux",
573+
uri.resolve("/qux").toString());
574+
}
575+
477576
main() {
478577
testUri("http:", true);
479578
testUri("file:///", true);
@@ -625,6 +724,7 @@ main() {
625724
testInvalidUrls();
626725
testNormalization();
627726
testReplace();
727+
testPackageUris();
628728
}
629729

630730
String dump(Uri uri) {

‎tests/corelib_2/uri_test.dart

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,105 @@ void testReplace() {
474474
Expect.listEquals(["43", "38"], params["y"]);
475475
}
476476

477+
void testPackageUris() {
478+
// A URI is recognized as a package URI if it has:
479+
// * "package" as scheme
480+
// * no authority
481+
// * a first path segment
482+
// * containing no `%`,
483+
// * which is not all "." characters,
484+
// * and which ends with a `/`.
485+
//
486+
// If so, the package name is unaffected by path resolution.
487+
var uri = Uri.parse("package:foo/bar/baz"); // Simple base URI.
488+
489+
Expect.stringEquals("package:foo/qux", // Resolve simple URI.
490+
uri.resolve("../../qux").toString());
491+
492+
Expect.stringEquals("package:foo/qux",
493+
uri.resolve("/qux").toString());
494+
495+
Expect.stringEquals("package:foo/qux?%2F", // Resolve non-simple URI.
496+
uri.resolve("../../qux?%2F").toString());
497+
498+
Expect.stringEquals("package:foo/qux?%2F",
499+
uri.resolve("/qux?%2F").toString());
500+
501+
uri = Uri.parse("package:foo/%62ar/baz"); // Non-simple base URI.
502+
503+
Expect.stringEquals("package:foo/qux", // Resolve simple URI.
504+
uri.resolve("../../qux").toString());
505+
506+
Expect.stringEquals("package:foo/qux",
507+
uri.resolve("/qux").toString());
508+
509+
Expect.stringEquals("package:foo/qux?%2F", // Resolve non-simple URI.
510+
uri.resolve("../../qux?%2F").toString());
511+
512+
Expect.stringEquals("package:foo/qux?%2F",
513+
uri.resolve("/qux?%2F").toString());
514+
515+
// The following base URIs are not recognized as package URIs:
516+
uri = Uri.parse("puckage:foo/bar/baz"); // Not "package" scheme.
517+
518+
Expect.stringEquals("puckage:/qux",
519+
uri.resolve("../../qux").toString());
520+
521+
Expect.stringEquals("puckage:/qux",
522+
uri.resolve("/qux").toString());
523+
524+
uri = Uri.parse("package://foo/bar/baz"); // Has authority.
525+
526+
Expect.stringEquals("package://foo/qux",
527+
uri.resolve("../../qux").toString());
528+
529+
Expect.stringEquals("package://foo/qux",
530+
uri.resolve("/qux").toString());
531+
532+
uri = Uri.parse("package:/foo/bar/baz"); // Has empty package name.
533+
534+
Expect.stringEquals("package:/qux",
535+
uri.resolve("../../qux").toString());
536+
537+
Expect.stringEquals("package:/qux",
538+
uri.resolve("/qux").toString());
539+
540+
uri = Uri.parse("package:f%2fo/bar/baz"); // Has escape in package name.
541+
542+
Expect.stringEquals("package:/qux",
543+
uri.resolve("../../qux").toString());
544+
545+
Expect.stringEquals("package:/qux",
546+
uri.resolve("/qux").toString());
547+
548+
uri = Uri.parse("package:f:o/bar/baz"); // Has colon in package name.
549+
550+
Expect.stringEquals("package:/qux",
551+
uri.resolve("../../qux").toString());
552+
553+
Expect.stringEquals("package:/qux",
554+
uri.resolve("/qux").toString());
555+
556+
uri = Uri.parse("package:.../bar/baz"); // Has only '.' in package name.
557+
558+
Expect.stringEquals("package:/qux",
559+
uri.resolve("../../qux").toString());
560+
561+
Expect.stringEquals("package:/qux",
562+
uri.resolve("/qux").toString());
563+
564+
uri = Uri.parse("package:foo?/"); // Has no `/` after package name.
565+
566+
// Resolving relative against non-absolute path gives
567+
// a non-absolute path again.
568+
// TODO(lrn): Is this a bug?
569+
Expect.stringEquals("package:qux",
570+
uri.resolve("../../qux").toString());
571+
572+
Expect.stringEquals("package:/qux",
573+
uri.resolve("/qux").toString());
574+
}
575+
477576
main() {
478577
testUri("http:", true);
479578
testUri("file:///", true);
@@ -625,6 +724,7 @@ main() {
625724
testInvalidUrls();
626725
testNormalization();
627726
testReplace();
727+
testPackageUris();
628728
}
629729

630730
String dump(Uri uri) {

0 commit comments

Comments
 (0)