Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit 1de6014

Browse files
lrhnCommit Queue
authored and
Commit Queue
committed
Fix bug in Uri's _normalizeRelativePath.
A leading empty segment would make the result look like an absolute path. This fix inserts a leading `./` before an otherwise leading empty segment. Added extensive tests. CoreLibraryReviewExempt: Internal code bug-fix, no API changes. Change-Id: I2bdac9bb6e0bd535af2dfcf33b3b0557e112e812 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/452142 Reviewed-by: Nate Bosch <nbosch@google.com> Commit-Queue: Lasse Nielsen <lrn@google.com>
1 parent 1d2b155 commit 1de6014

File tree

2 files changed

+243
-25
lines changed

2 files changed

+243
-25
lines changed

‎sdk/lib/core/uri.dart‎

Lines changed: 106 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2863,7 +2863,7 @@ final class _Uri implements _PlatformUri {
28632863
/// Normalizes using [_normalize] or returns substring of original.
28642864
///
28652865
/// If [_normalize] returns `null` (original content is already normalized),
2866-
/// this methods returns the substring if [component] from [start] to [end].
2866+
/// this methods returns the substring of [component] from [start] to [end].
28672867
static String _normalizeOrSubstring(
28682868
String component,
28692869
int start,
@@ -2994,7 +2994,7 @@ final class _Uri implements _PlatformUri {
29942994
}
29952995
int delta = baseEnd - newEnd;
29962996
// If we see a "." or ".." segment in base, stop here and let
2997-
// _removeDotSegments handle it.
2997+
// _removeDotSegments or _normalizeRelativePath handle it.
29982998
if ((delta == 2 || delta == 3) &&
29992999
base.codeUnitAt(newEnd + 1) == _DOT &&
30003000
(delta == 2 || base.codeUnitAt(newEnd + 2) == _DOT)) {
@@ -3053,45 +3053,132 @@ final class _Uri implements _PlatformUri {
30533053

30543054
/// Removes all `.` segments and any non-leading `..` segments.
30553055
///
3056-
/// If the path starts with something that looks like a scheme,
3057-
/// and [allowScheme] is false, the colon is escaped.
3058-
///
3059-
/// Removing the ".." from a "bar/foo/.." sequence results in "bar/"
3060-
/// (trailing "/"). If the entire path is removed (because it contains as
3061-
/// many ".." segments as real segments), the result is "./".
3062-
/// This is different from an empty string, which represents "no path"
3063-
/// when you resolve it against a base URI with a path with a non-empty
3064-
/// final segment.
3056+
/// Returns a new path that behaves the same as the input [path] when
3057+
/// used as a URI or URI Reference path.
3058+
///
3059+
/// If [allowScheme] is `false` and the path starts with something that would
3060+
/// parse as a scheme, valid scheme-characters followed by a colon,
3061+
/// the colon is escaped to `%3A`. The `allowScheme` should be set to `true`
3062+
/// when the result will become the path of a URI with no scheme or authority.
3063+
///
3064+
/// The result will contain no non-leading `..` segments. There may be
3065+
/// any number of leading `..` segments if the [path] contains more `..`
3066+
/// segments than prior actual segments. For example `a/b/../../../../c` will
3067+
/// normalize to `../../c`.
3068+
///
3069+
/// The result will contain no non-leading `.` segment.
3070+
/// It will only start with a `.` segment if necessary to preserve
3071+
/// the behavior of the input [path]:
3072+
/// * The result is the path `./` if the result would otherwise
3073+
/// have been an empty path, and the input `path` is not empty, or
3074+
/// * The result starts with `.//` to represent an otherwise leading
3075+
/// empty segment without being an absolute path.
3076+
///
3077+
/// The leading `.` segment
3078+
/// is only used to prefix a path that could otherwise behave differently
3079+
/// from the input [path]. An empty path means _no path_ in a URI, which
3080+
/// behaves differently from any non-empty path in some cases, and
3081+
/// URI paths cannot represent an empty leading path segment.
3082+
///
3083+
/// The result will end with a `/` if the [path] does, or if the path
3084+
/// ends with a `.` or `..` segment.
30653085
static String _normalizeRelativePath(String path, bool allowScheme) {
3066-
assert(!path.startsWith('/')); // Only get called for relative paths.
3086+
assert(!path.startsWith('/')); // Only get called with relative paths.
30673087
if (!_mayContainDotSegments(path)) {
30683088
if (!allowScheme) path = _escapeScheme(path);
30693089
return path;
30703090
}
30713091
assert(path.isNotEmpty); // An empty path would not have dot segments.
3092+
// Collects segments to be joined by `/` to produce the result.
3093+
// Used as a stack where `..` can pop the last segment.
30723094
List<String> output = [];
3095+
// Set to `true` after a `.` or `..` segment.
3096+
// If the path ends after a `.` or `..` with no trailing `/`,
3097+
// the result should have a trailing slash.
3098+
// For example "a/b" normalizes to "a/b", and "a/b/." normalizes to "a/b/",
3099+
// but the content of `output` will be the same in both cases.
3100+
// If the input ends with `appendSlash` set to `true`, the result will
3101+
// have an extra `/` added.
30733102
bool appendSlash = false;
3103+
// Split input into segments so `.` and `..` segments can be handled
3104+
// and remaining segments combined into a normalized path.
3105+
// The first segment will never be empty, since the path is relative
3106+
// and non-empty, so the path start with a non-`/` character.
3107+
// A final empty string does not semantically represent a path segment,
3108+
// just that the path ends in `/`.
3109+
// It's handled the same as any other empty segment.
3110+
// There is no difference between an empty segment and "no segment"
3111+
// unless followed by a `..` or `.`, which the final string isn't.
30743112
for (String segment in path.split("/")) {
3075-
appendSlash = false;
30763113
if (".." == segment) {
3114+
appendSlash = true;
30773115
if (!output.isEmpty && output.last != "..") {
30783116
output.removeLast();
3079-
appendSlash = true;
30803117
} else {
3081-
output.add("..");
3118+
output.add("..");// The result can have a number of _leading_ `..`s.
30823119
}
30833120
} else if ("." == segment) {
30843121
appendSlash = true;
30853122
} else {
3123+
appendSlash = false;
3124+
if (segment.isEmpty && output.isEmpty) {
3125+
// Avoid a leading empty segment, which would become an absolute path.
3126+
// Can occur for inputs like `.//etc` or `a/..//etc`.
3127+
//
3128+
// The returned path will be the `output` list joined with `/`s.
3129+
// A leading empty segment in `output` would make the resulting join
3130+
// start with `/` and be an absolute path,
3131+
// which is not a correct normalization of a relative path.
3132+
// Using `./` as "first segment" represents that empty segment as the
3133+
// segment between the `./` and the following joining `/`.
3134+
//
3135+
// The `./` first segment combines correctly with any following
3136+
// segments, whether `.`, `..` or empty/non-empty segments.
3137+
// * A `.` leaves the empty segment and sets `appendSlash` to true.
3138+
// * A `..` removes the `./` segment, effectively removing
3139+
// the empty segment between the `./` and the joining `/`
3140+
// and making the `output` empty. It sets `appendSlash` to true.
3141+
// * Any other segment is just appended and preserves the empty
3142+
// segment until an equal number of `..`s bring the `output`
3143+
// back to just the leading `"./"`.
3144+
// * If reaching the end of input with only the `./` in the `output`
3145+
// then either:
3146+
// * `appendSlash` is `true`, and the result becomes the path `.//`,
3147+
// which contains exactly the empty segment.
3148+
// * `appendSlash` is `false`, in which case the end of input
3149+
// was right after the empty segment that added the `./`,
3150+
// an empty segment that came from a trailing `/` in the input
3151+
// `path`. In that case, the empty segment shouldn't count
3152+
// as a real segment, which makes the result path be empty.
3153+
// In that case the result should be `./` anyway.
3154+
//
3155+
// In all cases, just treating `./` as a normal segment gives
3156+
// the desired behavior both when combined with later input segments
3157+
// and in the returned result.
3158+
//
3159+
// (This case could also be handled by checking at the end whether
3160+
// the first segment is empty, and if so, prepending the result with
3161+
// `"./"`. Simply changing the content of a first empty segment at
3162+
// this point is simple and cheap.)
3163+
segment = './';
3164+
}
30863165
output.add(segment);
30873166
}
30883167
}
3089-
if (output.isEmpty || (output.length == 1 && output[0].isEmpty)) {
3168+
if (output.isEmpty) {
3169+
// Can happen for, fx, `.` or `a/..`.
3170+
// A URI Reference does not distinguish having an empty path from having
3171+
// _no path_, and a URI Reference with no path behaves differently when
3172+
// combined with another URI or URI reference than one with non-empty
3173+
// path. To make this normalization not change the behavior, instead
3174+
// return a minimal non-empty normalized path.
30903175
return "./";
30913176
}
3092-
if (appendSlash || output.last == '..') output.add("");
3177+
assert(!output.first.isEmpty); // Would be `./` instead.
3178+
if (appendSlash) output.add("");
30933179
if (!allowScheme) output[0] = _escapeScheme(output[0]);
3094-
return output.join("/");
3180+
var result = output.join("/");
3181+
return result;
30953182
}
30963183

30973184
/// If [path] starts with a valid scheme, escape the percent.
@@ -3265,7 +3352,7 @@ final class _Uri implements _PlatformUri {
32653352
targetScheme = _makeScheme(targetScheme, 0, targetScheme.length);
32663353
}
32673354
if (split <= afterScheme) {
3268-
if (targetUserInfo!=null) {
3355+
if (targetUserInfo.isNotEmpty) {
32693356
targetUserInfo = _makeUserInfo(
32703357
targetUserInfo,
32713358
0,

‎tests/corelib/uri_test.dart‎

Lines changed: 137 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ library uriTest;
77
import "package:expect/expect.dart";
88
import 'dart:convert';
99

10-
testUri(String uriText, bool isAbsolute) {
10+
voidtestUri(String uriText, bool isAbsolute) {
1111
var uri = Uri.parse(uriText);
1212

1313
Expect.equals(isAbsolute, uri.isAbsolute);
@@ -29,21 +29,21 @@ testUri(String uriText, bool isAbsolute) {
2929
}
3030
}
3131

32-
testEncodeDecode(String orig, String encoded) {
32+
voidtestEncodeDecode(String orig, String encoded) {
3333
var e = Uri.encodeFull(orig);
3434
Expect.stringEquals(encoded, e);
3535
var d = Uri.decodeFull(encoded);
3636
Expect.stringEquals(orig, d);
3737
}
3838

39-
testEncodeDecodeComponent(String orig, String encoded) {
39+
voidtestEncodeDecodeComponent(String orig, String encoded) {
4040
var e = Uri.encodeComponent(orig);
4141
Expect.stringEquals(encoded, e);
4242
var d = Uri.decodeComponent(encoded);
4343
Expect.stringEquals(orig, d);
4444
}
4545

46-
testEncodeDecodeQueryComponent(
46+
voidtestEncodeDecodeQueryComponent(
4747
String orig,
4848
String encodedUTF8,
4949
String encodedLatin1,
@@ -77,7 +77,7 @@ testEncodeDecodeQueryComponent(
7777
}
7878
}
7979

80-
testUriPerRFCs() {
80+
voidtestUriPerRFCs() {
8181
final urisSample = "http://a/b/c/d;p?q";
8282
Uri base = Uri.parse(urisSample);
8383
testResolve(expect, relative) {
@@ -694,7 +694,7 @@ void testBackslashes() {
694694
);
695695
}
696696

697-
main() {
697+
voidmain() {
698698
testUri("http:", true);
699699
testUri("file:///", true);
700700
testUri("file", false);
@@ -878,6 +878,137 @@ main() {
878878
testBackslashes();
879879

880880
testNonBmpEncodingRegression();
881+
882+
testDotSlashSlashRegression();
883+
}
884+
885+
void testDotSlashSlashRegression() {
886+
// Regression test.
887+
//
888+
// A relative path must not normalize to one starting with an empty segment,
889+
// because that makes the result look like an absolute path.
890+
// To avoid that, prepend `./` before the leading empty segment.
891+
//
892+
// The `_normalizeRelativePath` misbehaved on inputs like `.//`, removing
893+
// the initial `.` segment and returning `/`.
894+
895+
// A replacement based implementation of path normalization, for comparison.
896+
//
897+
// (^|/).(/|$) -> (^|/)
898+
// (^|/)<segment>/..(/|$) -> (^|/)
899+
//
900+
// That is: Remove a `/./` or a `/segment/../` by turning it into a `/`,
901+
// unless it's at the start, then don't insert a leading `/`.
902+
// Add leading `./` if starting with `/` (starting with empty segment)
903+
// or has become empty.
904+
// Add final `/` if ending in `..` segment (and then also starting with
905+
// `..` segment).
906+
var normalizeRE = RegExp(r'(/|^)(?:\.|(?!\.\.(?:/|$))[^/]*/\.\.)(?:/|$)');
907+
String normalizeByReplacement(String path) {
908+
while (true) {
909+
var newPath = path.replaceAllMapped(normalizeRE, (m) => m[1]!);
910+
if (newPath == path) break;
911+
path = newPath;
912+
}
913+
// If ends with `..` segment, add `/` after it.
914+
if (path == '..' || path.endsWith('/..')) {
915+
path += '/';
916+
}
917+
// Don't return an absolute or empty path.
918+
if (path.isEmpty || path.startsWith('/')) path = "./$path";
919+
return path;
920+
}
921+
922+
// Check that `Uri.parse` gives the expected result.
923+
// Check that the expected result is what normalization by replacement
924+
// would give.
925+
void checkNormalize(String expectedPath, String input) {
926+
Expect.stringEquals(
927+
expectedPath,
928+
Uri.parse(input).toString(),
929+
"Uri.parse('$input')",
930+
);
931+
Expect.stringEquals(
932+
expectedPath,
933+
normalizeByReplacement(input),
934+
"NormByReplace('$input')",
935+
);
936+
}
937+
938+
var twoSlashRE = RegExp(r'(?<=/)(?=/)');
939+
void testNormalize(String expectedPath, String input) {
940+
checkNormalize(expectedPath, input);
941+
942+
// Change all empty segments, `//`, to `/x/`, in expectation and result.
943+
var nonEmptyExpect = expectedPath.replaceAll(twoSlashRE, 'x');
944+
if (nonEmptyExpect.startsWith('./x/')) {
945+
// Only differs if empty segment would have been first.
946+
nonEmptyExpect = nonEmptyExpect.substring(2);
947+
}
948+
checkNormalize(nonEmptyExpect, input.replaceAll(twoSlashRE, 'x'));
949+
}
950+
951+
const paths = [
952+
// (input, expected outout)
953+
('a/..', './'),
954+
('a/../', './'),
955+
('a/..//', './/'),
956+
('a/..//.', './/'),
957+
('a/..//./', './/'),
958+
('.', './'),
959+
('./', './'),
960+
('.//', './/'),
961+
('.//.', './/'),
962+
('.//./', './/'),
963+
('.//./.', './/'),
964+
('.//././', './/'),
965+
('.//./..', './'),
966+
('.//./../', './'),
967+
('.//.//.', './//'),
968+
('.//.//./', './//'),
969+
('.//', './/'),
970+
('.//a', './/a'),
971+
('.//..', './'),
972+
('.//../', './'),
973+
('.//../a', 'a'),
974+
('.//../a/', 'a/'),
975+
('.//.././', './'),
976+
('.//.././/', './/'),
977+
('.//../..', '../'),
978+
('.//../../', '../'),
979+
('.//../.././/', '..//'),
980+
('.//../..//', '..//'),
981+
('.//../..//..', '../'),
982+
('.//../..//../', '../'),
983+
('.//..//', './/'),
984+
('.//..//..', './'),
985+
('.//..//../', './'),
986+
('.///', './//'),
987+
('.///a', './//a'),
988+
('.///.', './//'),
989+
('.///./', './//'),
990+
('.///../..', './'),
991+
('.///../../', './'),
992+
// Not broken, extra sanity checks in case the fix broke it.
993+
('..', '../'),
994+
('../', '../'),
995+
('../a', '../a'),
996+
('../a/', '../a/'),
997+
('../.', '../'),
998+
('.././', '../'),
999+
('../..', '../../'),
1000+
('../../', '../../'),
1001+
('..//', '..//'),
1002+
('..//..', '../'),
1003+
('..//../', '../'),
1004+
('.././..', '../../'),
1005+
('.././/', '..//'),
1006+
];
1007+
for (var (input, expected) in paths) {
1008+
assert(!input.startsWith('/'));
1009+
testNormalize(expected, input);
1010+
testNormalize(expected, './$input'); // Prefixing with `./` changes nothing.
1011+
}
8811012
}
8821013

8831014
void testNonBmpEncodingRegression() {

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /