Skip to content

Commit

Permalink
[fixed] Do not decode + in pathname
Browse files Browse the repository at this point in the history
[removed] Path.encode/Path.decode

Fixes remix-run#716
  • Loading branch information
mjackson committed Jan 25, 2015
1 parent 8086698 commit 20c2c9b
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 45 deletions.
9 changes: 5 additions & 4 deletions modules/locations/HashLocation.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
var LocationActions = require('../actions/LocationActions');
var History = require('../History');
var Path = require('../utils/Path');

/**
* Returns the current URL path from the `hash` portion of the URL, including
* query string.
*/
function getHashPath() {
return Path.decode(
return decodeURI(
// We can't use window.location.hash here because it's not
// consistent across browsers - Firefox will pre-decode it!
window.location.href.split('#')[1] || ''
Expand Down Expand Up @@ -96,12 +95,14 @@ var HashLocation = {

push: function (path) {
_actionType = LocationActions.PUSH;
window.location.hash = Path.encode(path);
window.location.hash = encodeURI(path);
},

replace: function (path) {
_actionType = LocationActions.REPLACE;
window.location.replace(window.location.pathname + window.location.search + '#' + Path.encode(path));
window.location.replace(
window.location.pathname + window.location.search + '#' + encodeURI(path)
);
},

pop: function () {
Expand Down
7 changes: 3 additions & 4 deletions modules/locations/HistoryLocation.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
var LocationActions = require('../actions/LocationActions');
var History = require('../History');
var Path = require('../utils/Path');

/**
* Returns the current URL path from `window.location`, including query string.
*/
function getWindowPath() {
return Path.decode(
return decodeURI(
window.location.pathname + window.location.search
);
}
Expand Down Expand Up @@ -66,13 +65,13 @@ var HistoryLocation = {
},

push: function (path) {
window.history.pushState({ path: path }, '', Path.encode(path));
window.history.pushState({ path: path }, '', encodeURI(path));
History.length += 1;
notifyChange(LocationActions.PUSH);
},

replace: function (path) {
window.history.replaceState({ path: path }, '', Path.encode(path));
window.history.replaceState({ path: path }, '', encodeURI(path));
notifyChange(LocationActions.REPLACE);
},

Expand Down
5 changes: 2 additions & 3 deletions modules/locations/RefreshLocation.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
var HistoryLocation = require('./HistoryLocation');
var History = require('../History');
var Path = require('../utils/Path');

/**
* A Location that uses full page refreshes. This is used as
Expand All @@ -10,11 +9,11 @@ var Path = require('../utils/Path');
var RefreshLocation = {

push: function (path) {
window.location = Path.encode(path);
window.location = encodeURI(path);
},

replace: function (path) {
window.location.replace(Path.encode(path));
window.location.replace(encodeURI(path));
},

pop: History.back,
Expand Down
16 changes: 8 additions & 8 deletions modules/locations/__tests__/HashLocation-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,22 @@ var expect = require('expect');
var HashLocation = require('../HashLocation');

describe('HashLocation.getCurrentPath', function () {
afterEach(function () {
window.location.hash = '';
});

//this test is needed because Firefox will pre-decode the value retrieved from
//window.location.hash
it('returns a properly decoded equivalent of what window.location.hash is set to', function () {
it('returns a properly decoded equivalent of window.location.hash', function () {
window.location.hash = '';
expect(HashLocation.getCurrentPath()).toBe('');

window.location.hash = 'asdf';
expect(HashLocation.getCurrentPath()).toBe('asdf');

// + is only special in the query component, not the hash
window.location.hash = 'test+spaces';
expect(HashLocation.getCurrentPath()).toBe('test spaces');
expect(HashLocation.getCurrentPath()).toBe('test+spaces');

window.location.hash = 'first%2Fsecond';
expect(HashLocation.getCurrentPath()).toBe('first%2Fsecond');
Expand All @@ -24,7 +28,7 @@ describe('HashLocation.getCurrentPath', function () {
window.location.hash = 'first%252Fsecond';
expect(HashLocation.getCurrentPath()).toBe('first%2Fsecond');

//decodeURI doesn't handle lone percents
// decodeURI doesn't handle lone percents
window.location.hash = '%';
expect(function () {
HashLocation.getCurrentPath();
Expand All @@ -36,10 +40,6 @@ describe('HashLocation.getCurrentPath', function () {
window.location.hash =
'complicated+string/full%2Fof%3Fspecial%25chars%2520and%23escapes%E1%88%B4';
expect(HashLocation.getCurrentPath())
.toBe('complicated string/full%2Fof%3Fspecial%chars%20and%23escapesሴ');
});

afterEach(function () {
window.location.hash = '';
.toBe('complicated+string/full%2Fof%3Fspecial%chars%20and%23escapesሴ');
});
});
14 changes: 0 additions & 14 deletions modules/utils/Path.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,6 @@ function compilePattern(pattern) {

var Path = {

/**
* Safely decodes special characters in the given URL path.
*/
decode: function (path) {
return decodeURI(path.replace(/\+/g, ' '));
},

/**
* Safely encodes special characters in the given URL path.
*/
encode: function (path) {
return encodeURI(path).replace(/%20/g, '+');
},

/**
* Returns an array of the names of all parameters in the given pattern.
*/
Expand Down
12 changes: 0 additions & 12 deletions modules/utils/__tests__/Path-test.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,6 @@
var expect = require('expect');
var Path = require('../Path');

describe('Path.decode', function () {
it('properly decodes a path with a query string', function () {
expect(Path.decode('/my/short+path?a=b&c=d')).toEqual('/my/short path?a=b&c=d');
});
});

describe('Path.encode', function () {
it('properly encodes a path with a query string', function () {
expect(Path.encode('/my/short path?a=b&c=d')).toEqual('/my/short+path?a=b&c=d');
});
});

describe('Path.extractParamNames', function () {
describe('when a pattern contains no dynamic segments', function () {
it('returns an empty array', function () {
Expand Down

0 comments on commit 20c2c9b

Please sign in to comment.