Skip to content

Commit 9bd4406

Browse files
lrhnlrhn
committed
Fix bug in _SimpleUri.resolve.
In some cases it didn't follow our non-RFC behavior for resolving a relative path on top of another relative path. Fixes issue #27447 BUG= http://dartbug.com/27447 [email protected] Review URL: https://codereview.chromium.org/2374253004 .
1 parent 2ca7ae5 commit 9bd4406

File tree

2 files changed

+111
-56
lines changed

2 files changed

+111
-56
lines changed

‎sdk/lib/core/uri.dart

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4323,10 +4323,6 @@ class _SimpleUri implements Uri {
43234323
base._schemeCache);
43244324
}
43254325
// Merge paths.
4326-
if (base._uri.startsWith("../", base._pathStart)) {
4327-
// Complex rare case, go slow.
4328-
return _toNonSimple().resolveUri(ref);
4329-
}
43304326

43314327
// The RFC 3986 algorithm merges the base path without its final segment
43324328
// (anything after the final "/", or everything if the base path doesn't
@@ -4341,39 +4337,53 @@ class _SimpleUri implements Uri {
43414337
String refUri = ref._uri;
43424338
int baseStart = base._pathStart;
43434339
int baseEnd = base._queryStart;
4340+
while (baseUri.startsWith("../", baseStart)) baseStart += 3;
43444341
int refStart = ref._pathStart;
43454342
int refEnd = ref._queryStart;
4346-
int backCount = 1;
4347-
4348-
int slashCount = 0;
43494343

4350-
// Count leading ".." segments in reference path.
4344+
/// Count of leading ".." segments in reference path.
4345+
/// The count is decremented when the segment is matched with a
4346+
/// segment of the base path, and both are then omitted from the result.
4347+
int backCount = 0;
4348+
/// Count "../" segments and advance `refStart` to after the segments.
43514349
while (refStart + 3 <= refEnd && refUri.startsWith("../", refStart)) {
43524350
refStart += 3;
43534351
backCount += 1;
43544352
}
43554353

43564354
// Extra slash inserted between base and reference path parts if
4357-
// the base path contains any slashes.
4355+
// the base path contains any slashes, or empty string if none.
43584356
// (We could use a slash from the base path in most cases, but not if
43594357
// we remove the entire base path).
43604358
String insert = "";
4359+
4360+
/// Remove segments from the base path.
4361+
/// Start with the segment trailing the last slash,
4362+
/// then remove segments for each leading "../" segment
4363+
/// from the reference path, or as many of them as are available.
43614364
while (baseEnd > baseStart) {
43624365
baseEnd--;
43634366
int char = baseUri.codeUnitAt(baseEnd);
43644367
if (char == _SLASH) {
43654368
insert = "/";
4366-
backCount--;
43674369
if (backCount == 0) break;
4370+
backCount--;
43684371
}
43694372
}
4370-
// If the base URI has no scheme or authority (`_pathStart == 0`)
4371-
// and a relative path, and we reached the beginning of the path,
4372-
// we have a special case.
4373-
if (baseEnd == 0 && !base.hasAbsolutePath) {
4374-
// Non-RFC 3986 behavior when resolving a purely relative path on top of
4375-
// another relative path: Don't make the result absolute.
4373+
4374+
if (baseEnd == baseStart && !base.hasScheme && !base.hasAbsolutePath) {
4375+
// If the base is *just* a relative path (no scheme or authority),
4376+
// then merging with another relative path doesn't follow the
4377+
// RFC-3986 behavior.
4378+
// Don't need to check `base.hasAuthority` since the base path is
4379+
// non-empty - if there is an authority, a non-empty path is absolute.
4380+
4381+
// We reached the start of the base path, and want to stay relative,
4382+
// so don't insert a slash.
43764383
insert = "";
4384+
// If we reached the start of the base path with more "../" left over
4385+
// in the reference path, include those segments in the result.
4386+
refStart -= backCount * 3;
43774387
}
43784388

43794389
var delta = baseEnd - refStart + insert.length;

‎tests/corelib/uri_test.dart

Lines changed: 85 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,42 @@ testEncodeDecodeQueryComponent(String orig,
8585
}
8686

8787
testUriPerRFCs() {
88-
final urisSample = "http://a/b/c/d;p?q";
89-
Uri base = Uri.parse(urisSample);
88+
// Convert a Uri to a guaranteed "non simple" URI with the same content.
89+
toComplex(Uri uri) {
90+
Uri complex = new Uri(
91+
scheme: uri.scheme,
92+
userInfo: uri.hasAuthority ? uri.userInfo : null,
93+
host: uri.hasAuthority ? uri.host : null,
94+
port: uri.hasAuthority ? uri.port : null,
95+
path: uri.path,
96+
query: uri.hasQuery ? uri.query : null,
97+
fragment: uri.hasFragment ? uri.fragment : null,
98+
);
99+
assert(complex.toString() == uri.toString());
100+
return complex;
101+
}
102+
103+
Uri base;
104+
Uri complexBase;
105+
// Sets the [base] and [complexBase] to the parse of the URI and a
106+
// guaranteed non-simple version of the same URI.
107+
setBase(String uri) {
108+
base = Uri.parse(uri);
109+
complexBase = toComplex(base);
110+
}
111+
90112
testResolve(expect, relative) {
91113
String name = "$base << $relative";
92114
Expect.stringEquals(expect, base.resolve(relative).toString(), name);
115+
116+
Expect.stringEquals(expect, complexBase.resolve(relative).toString(),
117+
name + " (complex base)");
93118
}
94119

95120
// From RFC 3986.
121+
final urisSample = "http://a/b/c/d;p?q";
122+
setBase(urisSample);
123+
96124
testResolve("g:h", "g:h");
97125
testResolve("http://a/b/c/g", "g");
98126
testResolve("http://a/b/c/g", "./g");
@@ -139,47 +167,66 @@ testUriPerRFCs() {
139167
// Additional tests (not from RFC 3986).
140168
testResolve("http://a/b/g;p/h;s", "../g;p/h;s");
141169

142-
base = Uri.parse("s:a/b");
170+
setBase("s:a/b");
143171
testResolve("s:a/c", "c");
144172
testResolve("s:/c", "../c");
145173

146-
base = Uri.parse("S:a/b");
174+
setBase("S:a/b");
147175
testResolve("s:a/c", "c");
148176
testResolve("s:/c", "../c");
149177

150-
base = Uri.parse("s:foo");
178+
setBase("s:foo");
151179
testResolve("s:bar", "bar");
152180
testResolve("s:bar", "../bar");
153181

154-
base = Uri.parse("S:foo");
182+
setBase("S:foo");
155183
testResolve("s:bar", "bar");
156184
testResolve("s:bar", "../bar");
157185

158186
// Special-case (deliberate non-RFC behavior).
159-
base = Uri.parse("foo/bar");
187+
setBase("foo/bar");
160188
testResolve("foo/baz", "baz");
161189
testResolve("baz", "../baz");
162190

163-
base = Uri.parse("s:/foo");
191+
setBase("s:/foo");
164192
testResolve("s:/bar", "bar");
165193
testResolve("s:/bar", "../bar");
166194

167-
base = Uri.parse("S:/foo");
195+
setBase("S:/foo");
168196
testResolve("s:/bar", "bar");
169197
testResolve("s:/bar", "../bar");
170198

171199
// Test non-URI base (no scheme, no authority, relative path).
172-
base = Uri.parse("a/b/c?_#_");
200+
setBase("a/b/c?_#_");
173201
testResolve("a/b/g?q#f", "g?q#f");
174202
testResolve("./", "../..");
175203
testResolve("../", "../../..");
176204
testResolve("a/b/", ".");
177205
testResolve("c", "../../c"); // Deliberate non-RFC behavior.
178-
base = Uri.parse("../../a/b/c?_#_"); // Initial ".." in base url.
206+
setBase("../../a/b/c?_#_"); // Initial ".." in base url.
179207
testResolve("../../a/d", "../d");
208+
testResolve("../../d", "../../d");
180209
testResolve("../../../d", "../../../d");
181-
182-
base = Uri.parse("s://h/p?q#f"); // A simple base.
210+
setBase("../../a/b");
211+
testResolve("../../a/d", "d");
212+
testResolve("../../d", "../d");
213+
testResolve("../../../d", "../../d");
214+
setBase("../../a");
215+
testResolve("../../d", "d");
216+
testResolve("../../../d", "../d");
217+
testResolve("../../../../d", "../../d");
218+
219+
// Absoluyte path, not scheme or authority.
220+
setBase("/a");
221+
testResolve("/b", "b");
222+
testResolve("/b", "../b");
223+
testResolve("/b", "../../b");
224+
setBase("/a/b");
225+
testResolve("/a/c", "c");
226+
testResolve("/c", "../c");
227+
testResolve("/c", "../../c");
228+
229+
setBase("s://h/p?q#f"); // A simple base.
183230
// Simple references:
184231
testResolve("s2://h2/P?Q#F", "s2://h2/P?Q#F");
185232
testResolve("s://h2/P?Q#F", "//h2/P?Q#F");
@@ -195,7 +242,7 @@ testUriPerRFCs() {
195242
testResolve("s://h/p?Q#F%20", "?Q#F%20");
196243
testResolve("s://h/p?q#F%20", "#F%20");
197244

198-
base = Uri.parse("s://h/p1/p2/p3"); // A simple base with a path.
245+
setBase("s://h/p1/p2/p3"); // A simple base with a path.
199246
testResolve("s://h/p1/p2/", ".");
200247
testResolve("s://h/p1/p2/", "./");
201248
testResolve("s://h/p1/", "..");
@@ -206,7 +253,7 @@ testUriPerRFCs() {
206253
testResolve("s://h/", "../../../..");
207254
testResolve("s://h/", "../../../../");
208255

209-
base = Uri.parse("s://h/p?q#f%20"); // A non-simpe base.
256+
setBase("s://h/p?q#f%20"); // A non-simpe base.
210257
// Simple references:
211258
testResolve("s2://h2/P?Q#F", "s2://h2/P?Q#F");
212259
testResolve("s://h2/P?Q#F", "//h2/P?Q#F");
@@ -222,7 +269,7 @@ testUriPerRFCs() {
222269
testResolve("s://h/p?Q#F%20", "?Q#F%20");
223270
testResolve("s://h/p?q#F%20", "#F%20");
224271

225-
base = Uri.parse("S://h/p1/p2/p3"); // A non-simple base with a path.
272+
setBase("S://h/p1/p2/p3"); // A non-simple base with a path.
226273
testResolve("s://h/p1/p2/", ".");
227274
testResolve("s://h/p1/p2/", "./");
228275
testResolve("s://h/p1/", "..");
@@ -233,7 +280,7 @@ testUriPerRFCs() {
233280
testResolve("s://h/", "../../../..");
234281
testResolve("s://h/", "../../../../");
235282

236-
base = Uri.parse("../../../"); // A simple relative path.
283+
setBase("../../../"); // A simple relative path.
237284
testResolve("../../../a", "a");
238285
testResolve("../../../../a", "../a");
239286
testResolve("../../../a%20", "a%20");
@@ -242,8 +289,7 @@ testUriPerRFCs() {
242289
// Tests covering the branches of the merge algorithm in RFC 3986
243290
// with both simple and complex base URIs.
244291
for (var b in ["s://a/pa/pb?q#f", "s://a/pa/pb?q#f%20"]) {
245-
var origBase = Uri.parse(b);
246-
base = origBase;
292+
setBase(b);
247293

248294
// if defined(R.scheme) then ...
249295
testResolve("s2://a2/p2?q2#f2", "s2://a2/p2?q2#f2");
@@ -271,40 +317,40 @@ testUriPerRFCs() {
271317
// (Cover the merge function and the remove-dot-fragments functions too).
272318

273319
// If base has authority and empty path ...
274-
var emptyPathBase = Uri.parse(b.replaceFirst("/pa/pb", ""));
275-
base = emptyPathBase;
320+
var emptyPathBase = b.replaceFirst("/pa/pb", "");
321+
setBase(emptyPathBase);
276322
testResolve("s://a/p2?q2#f2", "p2?q2#f2");
277323
testResolve("s://a/p2#f2", "p2#f2");
278324
testResolve("s://a/p2", "p2");
279325

280-
base = origBase;
326+
setBase(b);
281327
// otherwise
282328
// (Cover both no authority and non-empty path and both).
283-
var noAuthEmptyPathBase = Uri.parse(b.replaceFirst("//a/pa/pb", ""));
284-
var noAuthAbsPathBase = Uri.parse(b.replaceFirst("//a", ""));
285-
var noAuthRelPathBase = Uri.parse(b.replaceFirst("//a/", ""));
286-
var noAuthRelSinglePathBase = Uri.parse(b.replaceFirst("//a/pa/", ""));
329+
var noAuthEmptyPathBase = b.replaceFirst("//a/pa/pb", "");
330+
var noAuthAbsPathBase = b.replaceFirst("//a", "");
331+
var noAuthRelPathBase = b.replaceFirst("//a/", "");
332+
var noAuthRelSinglePathBase = b.replaceFirst("//a/pa/", "");
287333

288334
testResolve("s://a/pa/p2?q2#f2", "p2?q2#f2");
289335
testResolve("s://a/pa/p2#f2", "p2#f2");
290336
testResolve("s://a/pa/p2", "p2");
291337

292-
base = noAuthEmptyPathBase;
338+
setBase(noAuthEmptyPathBase);
293339
testResolve("s:p2?q2#f2", "p2?q2#f2");
294340
testResolve("s:p2#f2", "p2#f2");
295341
testResolve("s:p2", "p2");
296342

297-
base = noAuthAbsPathBase;
343+
setBase(noAuthAbsPathBase);
298344
testResolve("s:/pa/p2?q2#f2", "p2?q2#f2");
299345
testResolve("s:/pa/p2#f2", "p2#f2");
300346
testResolve("s:/pa/p2", "p2");
301347

302-
base = noAuthRelPathBase;
348+
setBase(noAuthRelPathBase);
303349
testResolve("s:pa/p2?q2#f2", "p2?q2#f2");
304350
testResolve("s:pa/p2#f2", "p2#f2");
305351
testResolve("s:pa/p2", "p2");
306352

307-
base = noAuthRelSinglePathBase;
353+
setBase(noAuthRelSinglePathBase);
308354
testResolve("s:p2?q2#f2", "p2?q2#f2");
309355
testResolve("s:p2#f2", "p2#f2");
310356
testResolve("s:p2", "p2");
@@ -314,7 +360,7 @@ testUriPerRFCs() {
314360
// A. if input buffer starts with "../" or "./".
315361
// This only happens if base has only a single (may be empty) segment and
316362
// no slash.
317-
base = emptyPathBase;
363+
setBase(emptyPathBase);
318364
testResolve("s://a/p2", "../p2");
319365
testResolve("s://a/", "../");
320366
testResolve("s://a/", "..");
@@ -324,7 +370,7 @@ testUriPerRFCs() {
324370
testResolve("s://a/p2", "../../p2");
325371
testResolve("s://a/p2", "../../././p2");
326372

327-
base = noAuthRelSinglePathBase;
373+
setBase(noAuthRelSinglePathBase);
328374
testResolve("s:p2", "../p2");
329375
testResolve("s:", "../");
330376
testResolve("s:", "..");
@@ -337,31 +383,30 @@ testUriPerRFCs() {
337383
// B. if input buffer starts with "/./" or is "/.". replace with "/".
338384
// (The URI implementation removes the "." path segments when parsing,
339385
// so this case isn't handled by merge).
340-
base = origBase;
386+
setBase(b);
341387
testResolve("s://a/pa/p2", "./p2");
342388

343389
// C. if input buffer starts with "/../" or is "/..", replace with "/"
344390
// and remove preceeding segment.
345391
testResolve("s://a/p2", "../p2");
346-
var longPathBase = Uri.parse(b.replaceFirst("/pb", "/pb/pc/pd"));
347-
base = longPathBase;
392+
var longPathBase = b.replaceFirst("/pb", "/pb/pc/pd");
393+
setBase(longPathBase);
348394
testResolve("s://a/pa/pb/p2", "../p2");
349395
testResolve("s://a/pa/p2", "../../p2");
350396
testResolve("s://a/p2", "../../../p2");
351397
testResolve("s://a/p2", "../../../../p2");
352-
var noAuthRelLongPathBase =
353-
Uri.parse(b.replaceFirst("//a/pa/pb", "pa/pb/pc/pd"));
354-
base = noAuthRelLongPathBase;
398+
var noAuthRelLongPathBase = b.replaceFirst("//a/pa/pb", "pa/pb/pc/pd");
399+
setBase(noAuthRelLongPathBase);
355400
testResolve("s:pa/pb/p2", "../p2");
356401
testResolve("s:pa/p2", "../../p2");
357402
testResolve("s:/p2", "../../../p2");
358403
testResolve("s:/p2", "../../../../p2");
359404

360405
// D. if the input buffer contains only ".." or ".", remove it.
361-
base = noAuthEmptyPathBase;
406+
setBase(noAuthEmptyPathBase);
362407
testResolve("s:", "..");
363408
testResolve("s:", ".");
364-
base = noAuthRelSinglePathBase;
409+
setBase(noAuthRelSinglePathBase);
365410
testResolve("s:", "..");
366411
testResolve("s:", ".");
367412
}

0 commit comments

Comments
 (0)