diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js index eece232ac0..8f53cf0157 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js @@ -383,12 +383,16 @@ _computeDiffURL: function(changeNum, patchRange, path) { // @see Issue 4255 regarding double-encoding. + path = encodeURIComponent(encodeURIComponent(path)); + // @see Issue 4577 regarding more readable URLs. + path = path.replace(/%252F/g, '/'); + path = path.replace(/%2520/g, '+'); return '/c/' + encodeURIComponent(changeNum) + '/' + encodeURIComponent(this._patchRangeStr(patchRange)) + '/' + - encodeURIComponent(encodeURIComponent(path)); + path; }, _patchRangeStr: function(patchRange) { diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html index 3fe785e5b2..76aceef586 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html @@ -39,6 +39,7 @@ limitations under the License. setup(function() { stub('gr-rest-api-interface', { getLoggedIn: function() { return Promise.resolve(true); }, + getPreferences: function() { return Promise.resolve({}); }, }); element = fixture('basic'); }); @@ -312,9 +313,9 @@ limitations under the License. diffStub.restore(); }); - test('file name should be double-escaped', function() { + test('path should be properly escaped', function() { element._files = [ - {__path: 'my+file.txt'}, + {__path: 'foo bar/my+file.txt%'}, ]; element.changeNum = '42'; element.patchRange = { @@ -322,8 +323,12 @@ limitations under the License. patchNum: '2', }; flushAsynchronousOperations(); + // Slashes should be preserved, and spaces should be translated to `+`. + // @see Issue 4255 regarding double-encoding. + // @see Issue 4577 regarding more readable URLs. assert.equal( - element.$$('a').getAttribute('href'), '/c/42/2/my%252Bfile.txt'); + element.$$('a').getAttribute('href'), + '/c/42/2/foo+bar/my%252Bfile.txt%2525'); }); }); diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js index 1ddff39c9d..a182f8a45b 100644 --- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js +++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js @@ -135,12 +135,17 @@ // Don't allow diffing the same patch number against itself. if (params.basePatchNum === params.patchNum) { // @see Issue 4255 regarding double-encoding. + var path = encodeURIComponent(encodeURIComponent(path)); + // @see Issue 4577 regarding more readable URLs. + path = path.replace(/%252F/g, '/'); + path = path.replace(/%2520/g, '+'); + page.redirect('/c/' + encodeURIComponent(params.changeNum) + '/' + encodeURIComponent(params.patchNum) + '/' + - encodeURIComponent(encodeURIComponent(params.path))); + path); return; }