Skip to content
This repository was archived by the owner on Oct 8, 2021. It is now read-only.

Commit e412102

Browse files
author
Gabriel Schulhof
committed
Navigation: Correctly (un)escape data-url all throughout the code
Also removes code dealing with nested list URL tokens (subPageUrlKey) The initial bug was regarding the need to use selector-escaping when looking for pages inside the DOM via their data-url attribute, because the the data-url may contain "weird" characters like parantheses, single, and double quotes. During the process of fixing this it became clear that the data-url attribute can sometimes end up having a URL-encoded value - but not always. This makes proper string comparison, much less selector-escaping, impossible. So, it became necessary to require that data-url never be url-encoded. Conversely, the URL that ends up in the location when using replaceState() must be URL-encoded, otherwise, upon deep linking, one may end up with an invalid URL. For example, if the URL contains an actual percent sign, and the URL is placed in the location via replaceState(), the percent sign must be URL-encoded in order for deep-linking to work. Below are the original commit messages that have been squashed: Pagecontainer: Escape dataUrl when trying to find page based on it Navigation: Make sure location is encoded when going to a funky page Pagecontainer: Test that _find() throws not when looking for weird URLs Init: Correctly assign data-url for initial page Init: Make sure "data-url" attribute for initial page is decoded Pagecontainer: Decode URI when assigning to "data-url" during _parse() Pagecontainer: Use decoded URL to build selector for data-url Pagecontainer: Call convertUrlToDataUrl() via _createDataUrl() Path: Always uri-decode when computing dataUrl Pagecontainer: Remove extra decoding step, now part of _createDataUrl() This also removes the possibility that a URL gets double-decoded. Pagecontainer: Test behavior of _find() in the face of weird URLs Navigation: Use data-url retrieved from Ajax request in its encoded form Pagecontainer: Pass encoded URL to $.mobile.navigate() Init: Rename JS file with spaces in it Closes gh-7474 Fixes gh-1383
1 parent 27d808a commit e412102

File tree

12 files changed

+180
-25
lines changed

12 files changed

+180
-25
lines changed

js/init.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ define([
7171
// unless the data url is already set set it to the pathname
7272
if ( !$this[ 0 ].getAttribute( "data-" + $.mobile.ns + "url" ) ) {
7373
$this.attr( "data-" + $.mobile.ns + "url", $this.attr( "id" ) ||
74-
theLocation.pathname + theLocation.search );
74+
path.convertUrlToDataUrl( theLocation.pathname + theLocation.search ) );
7575
}
7676
});
7777

js/navigation/path.js

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -202,19 +202,21 @@ define([
202202
},
203203

204204
convertUrlToDataUrl: function( absUrl ) {
205-
var u = path.parseUrl( absUrl );
205+
var result = absUrl,
206+
u = path.parseUrl( absUrl );
207+
206208
if ( path.isEmbeddedPage( u ) ) {
207209
// For embedded pages, remove the dialog hash key as in getFilePath(),
208210
// and remove otherwise the Data Url won't match the id of the embedded Page.
209-
return u.hash
211+
result = u.hash
210212
.split( dialogHashKey )[0]
211213
.replace( /^#/, "" )
212214
.replace( /\?.*$/, "" );
213215
} else if ( path.isSameDomain( u, this.documentBase ) ) {
214-
return u.hrefNoHash.replace( this.documentBase.domain, "" ).split( dialogHashKey )[0];
216+
result = u.hrefNoHash.replace( this.documentBase.domain, "" ).split( dialogHashKey )[0];
215217
}
216218

217-
return window.decodeURIComponent(absUrl);
219+
return window.decodeURIComponent( result );
218220
},
219221

220222
//get path from current hash, or from a file path
@@ -374,11 +376,10 @@ define([
374376
return ( hasHash ? "#" : "" ) + hash.replace( /([!"#$%&'()*+,./:;<=>?@[\]^`{|}~])/g, "\\$1" );
375377
},
376378

377-
// return the substring of a filepath before the sub-page key, for making
378-
// a server request
379+
// return the substring of a filepath before the dialogHashKey, for making a server
380+
// request
379381
getFilePath: function( path ) {
380-
var splitkey = "&" + $.mobile.subPageUrlKey;
381-
return path && path.split( splitkey )[0].split( dialogHashKey )[0];
382+
return path && path.split( dialogHashKey )[0];
382383
},
383384

384385
// check if the specified url refers to the first page in the main

js/widgets/pagecontainer.js

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,8 @@ define( [
362362
// NOTE do _not_ use the :jqmData pseudo selector because parenthesis
363363
// are a valid url char and it breaks on the first occurence
364364
page = this.element
365-
.children( "[data-" + this._getNs() +"url='" + dataUrl + "']" );
365+
.children( "[data-" + this._getNs() +
366+
"url='" + $.mobile.path.hashToSelector( dataUrl ) + "']" );
366367

367368
// If we failed to find the page, check to see if the url is a
368369
// reference to an embedded page. If so, it may have been dynamically
@@ -447,7 +448,7 @@ define( [
447448
// TODO tagging a page with external to make sure that embedded pages aren't
448449
// removed by the various page handling code is bad. Having page handling code
449450
// in many places is bad. Solutions post 1.0
450-
page.attr( "data-" + this._getNs() + "url", $.mobile.path.convertUrlToDataUrl(fileUrl) )
451+
page.attr( "data-" + this._getNs() + "url", this._createDataUrl( fileUrl ) )
451452
.attr( "data-" + this._getNs() + "external-page", true );
452453

453454
return page;
@@ -496,8 +497,7 @@ define( [
496497
// or require ordering such that other bits are sprinkled in between parts that
497498
// could be abstracted out as a group
498499
_loadSuccess: function( absUrl, triggerData, settings, deferred ) {
499-
var fileUrl = this._createFileUrl( absUrl ),
500-
dataUrl = this._createDataUrl( absUrl );
500+
var fileUrl = this._createFileUrl( absUrl );
501501

502502
return $.proxy(function( html, textStatus, xhr ) {
503503
//pre-parse html to check for a data-url,
@@ -517,6 +517,11 @@ define( [
517517
dataUrlRegex.test( RegExp.$1 ) &&
518518
RegExp.$1 ) {
519519
fileUrl = $.mobile.path.getFilePath( $("<div>" + RegExp.$1 + "</div>").text() );
520+
521+
// We specify that, if a data-url attribute is given on the page div, its value
522+
// must be given non-URL-encoded. However, in this part of the code, fileUrl is
523+
// assumed to be URL-encoded, so we URL-encode the retrieved value here
524+
fileUrl = this.window[ 0 ].encodeURIComponent( fileUrl );
520525
}
521526

522527
//dont update the base tag if we are prefetching
@@ -554,13 +559,6 @@ define( [
554559

555560
this._include( content, settings );
556561

557-
// Enhancing the content may result in new dialogs/sub content being inserted
558-
// into the DOM. If the original absUrl refers to a sub-content, that is the
559-
// real content we are interested in.
560-
if ( absUrl.indexOf( "&" + $.mobile.subPageUrlKey ) > -1 ) {
561-
content = this.element.children( "[data-" + this._getNs() +"url='" + dataUrl + "']" );
562-
}
563-
564562
// Remove loading message.
565563
if ( settings.showLoadMsg ) {
566564
this._hideLoading();
@@ -1131,7 +1129,7 @@ define( [
11311129
};
11321130

11331131
if ( settings.changeHash !== false && $.mobile.hashListeningEnabled ) {
1134-
$.mobile.navigate( url, params, true);
1132+
$.mobile.navigate( this.window[ 0 ].encodeURI( url ), params, true);
11351133
} else if ( toPage[ 0 ] !== $.mobile.firstPage[ 0 ] ) {
11361134
$.mobile.navigate.history.add( url, params );
11371135
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<!doctype html>
2+
<html>
3+
<head>
4+
</head>
5+
<body>
6+
<div data-nstest-role="page"><div id="percentPageChild"></div></div>
7+
</body>
8+
</html>

tests/integration/navigation/index.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
<div id="harmless-default-page" data-nstest-role="page" class="first-page">
3333
<a id="goToGoogle" href="http://www.google.com/#abc">Go to Google</a>
34+
<a id="goToPercentPage" href="100%25test/behind-the-percent.html">Go to percent page</a>
3435
</div>
3536

3637
<div id="foo" data-nstest-role="page" class="foo-class">

tests/integration/navigation/navigation_core.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,49 @@ $.testHelper.delayStart();
3636
"Default is not prevented when clicking on external link with hash" );
3737
});
3838

39+
( function() {
40+
41+
var goUrl, originalGo,
42+
navigatorPrototype = $.mobile.Navigator.prototype;
43+
44+
module( "Navigation encoding", {
45+
setup: function() {
46+
goUrl = undefined;
47+
originalGo = navigatorPrototype.go;
48+
navigatorPrototype.go = function( url ) {
49+
goUrl = url;
50+
51+
return originalGo.apply( this, arguments );
52+
};
53+
},
54+
teardown: function() {
55+
navigatorPrototype.go = originalGo;
56+
}
57+
});
58+
59+
asyncTest( "Going to a page requiring url encoding works", function() {
60+
var endingString = $( "#goToPercentPage" ).attr( "href" );
61+
62+
$.testHelper.pageSequence([
63+
function() {
64+
$( "#goToPercentPage" ).click();
65+
},
66+
function() {
67+
deepEqual( $.mobile.activePage.children( "#percentPageChild" ).length, 1,
68+
"Active page is the one loaded from the directory with a percent symbol" );
69+
70+
deepEqual(
71+
goUrl.lastIndexOf( endingString ),
72+
goUrl.length - endingString.length,
73+
"Location ends in '" + endingString + "'" );
74+
$.mobile.back();
75+
},
76+
start
77+
]);
78+
});
79+
80+
})();
81+
3982
module('jquery.mobile.navigation.js', {
4083
setup: function() {
4184
$.mobile.navigate.history.stack = [];

tests/unit/content/content_core.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,4 +512,23 @@
512512

513513
proto._loadUrl( "foo", {}, {} );
514514
});
515+
516+
test( "_find() does not throw upon encountering a weird file name", function() {
517+
var errorThrown,
518+
proto = $.mobile.pagecontainer.prototype;
519+
520+
try {
521+
proto._find.call({
522+
_getNs: proto._getNs,
523+
_createFileUrl: proto._createFileUrl,
524+
_createDataUrl: proto._createDataUrl,
525+
_getInitialContent: function() { return $( "<div>" ); },
526+
element: $( "<body>" )
527+
}, "http://localhost/Raison d'être.html" );
528+
} catch( error ) {
529+
errorThrown = error;
530+
}
531+
532+
deepEqual( errorThrown, undefined, "Error was not thrown" );
533+
});
515534
})(jQuery);
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<title>jQuery Mobile Init Test Suite</title>
6+
7+
<script src="../../../external/requirejs/require.js"></script>
8+
<script src="../../../js/requirejs.config.js"></script>
9+
<script src="../../requirejs.config.js"></script>
10+
<script src="../../../js/jquery.tag.inserter.js"></script>
11+
<script src="../../jquery.setNameSpace.js"></script>
12+
<script src="../../jquery.testHelper.js"></script>
13+
14+
<link rel="stylesheet" href="../../../css/themes/default/jquery.mobile.css" />
15+
<link rel="stylesheet" href="../../../external/qunit/qunit.css"/>
16+
<link rel="stylesheet" href="../../jqm-tests.css"/>
17+
<script src="../../../external/qunit/qunit.js"></script>
18+
<script>
19+
$.testHelper.asyncLoad([
20+
[ "weird_file_name_core.js" ],
21+
[ "init" ]
22+
]);
23+
</script>
24+
25+
<script src="../../swarminject.js"></script>
26+
</head>
27+
<body>
28+
29+
<div id="qunit"></div>
30+
31+
<div data-nstest-role="page"></div>
32+
</body>
33+
</html>
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
( function() {
2+
3+
$( document ).on( "mobileinit", function() {
4+
var origParseLocation = $.mobile.path.parseLocation,
5+
origInitializePage = $.mobile.initializePage;
6+
7+
// Overwrite parseLocation with a version that always urlencodes the name of the tests file
8+
$.mobile.path.parseLocation = function() {
9+
var parsedLocation = origParseLocation.apply( this, arguments ),
10+
returnValue = {};
11+
12+
// Make sure the location bits appear urlEncoded
13+
$.each( parsedLocation, function( key, value ) {
14+
returnValue[ key ] =
15+
value.replace( /weird file name-tests/g, "weird%20file%20name-tests" );
16+
});
17+
18+
return returnValue;
19+
};
20+
21+
// Overwrite initializePage with a version that restores both itself and parseLocation after
22+
// one call to itself
23+
$.mobile.initializePage = function() {
24+
25+
$.mobile.intializePage = origInitializePage;
26+
$.mobile.path.parseLocation = origParseLocation;
27+
28+
return origInitializePage.apply( this, arguments );
29+
};
30+
});
31+
32+
test( "data-url for initial page is urldecoded", function() {
33+
deepEqual(
34+
!!$( ":mobile-page" )
35+
.attr( "data-" + $.mobile.ns + "url" )
36+
.match( /weird%20file%20name/ ),
37+
false,
38+
"Value of 'data-url' attribute is not urlencoded" );
39+
});
40+
41+
})();

tests/unit/pagecontainer/index.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,6 @@
3636
<a href="page-does-not-exist.html" id="go-to-nonexistent-page">Go to non-existent page</a>
3737
</div>
3838
</div>
39+
<div data-nstest-role="page" data-nstest-url="Raison d'être.html" class="weird-data-url"></div>
3940
</body>
4041
</html>

tests/unit/pagecontainer/pagecontainer_core.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
test( "_find() can handle weird data-url attributes", function() {
2+
deepEqual(
3+
$.mobile.pagecontainer.prototype._find.call({
4+
_createFileUrl: $.mobile.pagecontainer.prototype._createFileUrl,
5+
_createDataUrl: $.mobile.pagecontainer.prototype._createDataUrl,
6+
_getInitialContent: $.mobile.pagecontainer.prototype._getInitialContent,
7+
element: $( "body" ),
8+
_getNs: $.mobile.pagecontainer.prototype._getNs,
9+
10+
}, "Raison d'être.html" )[ 0 ],
11+
$( ".weird-data-url" )[ 0 ],
12+
"Correct element is retrieved when the file name is weird" );
13+
});
14+
115
( function() {
216
var originalLoad = $.mobile.pagecontainer.prototype._triggerWithDeprecated
317
module( "load method", {

tests/unit/path/path_core.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,6 @@
2929
ok($.mobile.path.isPath('/'), "anything with a slash is a path");
3030
});
3131

32-
test( "path.getFilePath method is working properly", function(){
33-
deepEqual($.mobile.path.getFilePath("foo.html" + "&" + $.mobile.subPageUrlKey ), "foo.html", "returns path without sub page key");
34-
});
35-
3632
test( "path.set method is working properly", function(){
3733
$.mobile.navigate.history.ignoreNextHashChange = false;
3834
$.mobile.path.set("foo");

0 commit comments

Comments
 (0)