opendev/gerrit
Code Issues Proposed changes

Decode inheritance project IDs before use

Browse Source 
In the JSON of emitted by the repo list endpoint, the project IDs are
URL-encoded using java.net.URLEncoder.encode. While the Java function
encodes spaces as plus-signs, the JavaScript decodeURLComponent does
not.

As a result, when editing the "inherits from" field of a project's
access settings where the parent project's ID includes spaces, the
payload would use an improperly decoded parent value, and this would be
rejected by the API for failing to identify the parent.

Add a new decoding method to the URL encoding behavior that better
matches the style of encoding used by java.net.URLEncoder. Because the
behavior had no test coverage before, it's moved to a new location and
a suite of tests are added. Add tests for the access screen properly
encoding and decoding parent IDs.

Change-Id: Icb1d28d8f1f88c3c7373aa1c8953238a61c32ace
This commit is contained in:
Wyatt Allen
2018-07-30 17:14:49 -07:00
parent 8566553652
commit 292d013f32
22 changed files with 153 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
polygerrit-ui/app/behaviors/gr-list-view-behavior/gr-list-view-behavior.html
View File

@@ -15,7 +15,7 @@ See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="../base-url-behavior/base-url-behavior.html">
<link rel="import" href="../gr-url-encoding-behavior.html">
<link rel="import" href="../gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<script>
(function(window) {
'use strict';

15
polygerrit-ui/app/behaviors/gr-url-encoding-behavior.html → polygerrit-ui/app/behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html
View File

@@ -25,6 +25,9 @@ limitations under the License.
/**
* Pretty-encodes a URL. Double-encodes the string, and then replaces
* benevolent characters for legibility.
* @param {string} url
* @param {boolean=} replaceSlashes
* @return {string}
*/
encodeURL(url, replaceSlashes) {
// @see Issue 4255 regarding double-encoding.
@@ -37,6 +40,18 @@ limitations under the License.
}
return output;
},
/**
* Single decode for URL components. Will decode plus signs ('+') to spaces.
* Note: because this function decodes once, it is not the inverse of
* encodeURL.
* @param {string} url
* @return {string}
*/
singleDecodeURL(url) {
const withoutPlus = url.replace(/\+/g, '%20');
return decodeURIComponent(withoutPlus);
},
};
})(window);
</script>

91
polygerrit-ui/app/behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior_test.html Normal file
View File

@@ -0,0 +1,91 @@
<!DOCTYPE html>
<!--
@license
Copyright (C) 2018 The Android Open Source Project
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<title>gr-url-encoding-behavior</title>
<script src="../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
<script src="../../bower_components/web-component-tester/browser.js"></script>
<link rel="import" href="../../test/common-test-setup.html"/>
<link rel="import" href="gr-url-encoding-behavior.html">
<script>void(0);</script>
<test-fixture id="basic">
<template>
<test-element></test-element>
</template>
</test-fixture>
<script>
suite('gr-url-encoding-behavior tests', () => {
let element;
let sandbox;
suiteSetup(() => {
// Define a Polymer element that uses this behavior.
Polymer({
is: 'test-element',
behaviors: [Gerrit.URLEncodingBehavior],
});
});
setup(() => {
sandbox = sinon.sandbox.create();
element = fixture('basic');
});
teardown(() => {
sandbox.restore();
});
suite('encodeURL', () => {
test('double encodes', () => {
assert.equal(element.encodeURL('abc?123'), 'abc%253F123');
assert.equal(element.encodeURL('def/ghi'), 'def%252Fghi');
assert.equal(element.encodeURL('jkl'), 'jkl');
assert.equal(element.encodeURL(''), '');
});
test('does not convert colons', () => {
assert.equal(element.encodeURL('mno:pqr'), 'mno:pqr');
});
test('converts spaces to +', () => {
assert.equal(element.encodeURL('words with spaces'), 'words+with+spaces');
});
test('does not convert slashes when configured', () => {
assert.equal(element.encodeURL('stu/vwx', true), 'stu/vwx');
});
test('does not convert slashes when configured', () => {
assert.equal(element.encodeURL('stu/vwx', true), 'stu/vwx');
});
});
suite('singleDecodeUrl', () => {
test('single decodes', () => {
assert.equal(element.singleDecodeURL('abc%3Fdef'), 'abc?def');
});
test('converts + to space', () => {
assert.equal(element.singleDecodeURL('ghi+jkl'), 'ghi jkl');
});
});
});
</script>

2
polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.html
View File

@@ -19,7 +19,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-admin-nav-behavior/gr-admin-nav-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../styles/gr-menu-page-styles.html">
<link rel="import" href="../../../styles/gr-page-nav-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">

2
polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
View File

@@ -19,7 +19,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">

2
polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.html
View File

@@ -18,7 +18,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">

2
polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html
View File

@@ -18,7 +18,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">

2
polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.html
View File

@@ -18,7 +18,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">

2
polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.html
View File

@@ -16,7 +16,7 @@ limitations under the License.
-->
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../bower_components/iron-autogrow-textarea/iron-autogrow-textarea.html">
<link rel="import" href="../../../bower_components/iron-input/iron-input.html">

2
polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.html
View File

@@ -19,7 +19,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/gr-access-behavior/gr-access-behavior.html">
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../styles/gr-menu-page-styles.html">
<link rel="import" href="../../../styles/gr-subpage-styles.html">

21
polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.js
View File

@@ -196,11 +196,10 @@
},
_handleUpdateInheritFrom(e) {
const projectId = decodeURIComponent(e.detail.value);
if (!this._inheritsFrom) {
this._inheritsFrom = {};
}
this._inheritsFrom.id = projectId;
this._inheritsFrom.id = e.detail.value;
this._inheritsFrom.name = this._inheritFromFilter;
this._handleAccessModified();
},
@@ -361,18 +360,24 @@
remove: {},
};
const originalInheritsFromId = this._originalInheritsFrom ?
this.singleDecodeURL(this._originalInheritsFrom.id) :
null;
const inheritsFromId = this._inheritsFrom ?
this.singleDecodeURL(this._inheritsFrom.id) :
null;
const inheritFromChanged =
// Inherit from changed
(this._originalInheritsFrom &&
this._originalInheritsFrom.id !== this._inheritsFrom.id) ||
// Inherit froma dded (did not have one initially);
(!this._originalInheritsFrom && this._inheritsFrom
&& this._inheritsFrom.id);
(originalInheritsFromId
&& originalInheritsFromId !== inheritsFromId) ||
// Inherit from added (did not have one initially);
(!originalInheritsFromId && inheritsFromId);
this._recursivelyUpdateAddRemoveObj(this._local, addRemoveObj);
if (inheritFromChanged) {
addRemoveObj.parent = this._inheritsFrom.id;
addRemoveObj.parent = inheritsFromId;
}
return addRemoveObj;
},

16
polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access_test.html
View File

@@ -238,6 +238,14 @@ limitations under the License.
'none');
});
test('_handleUpdateInheritFrom', () => {
element._inheritFromFilter = 'foo bar baz';
element._handleUpdateInheritFrom({detail: {value: 'abc+123'}});
assert.isOk(element._inheritsFrom);
assert.equal(element._inheritsFrom.id, 'abc+123');
assert.equal(element._inheritsFrom.name, 'foo bar baz');
});
test('_computeLoadingClass', () => {
assert.equal(element._computeLoadingClass(true), 'loading');
assert.equal(element._computeLoadingClass(false), '');
@@ -422,6 +430,14 @@ limitations under the License.
});
});
test('_handleSaveForReview new parent with spaces', () => {
element._inheritsFrom = {id: 'spaces+in+project+name'};
element._originalInheritsFrom = {id: 'old-project'};
assert.deepEqual(element._computeAddAndRemove(), {
parent: 'spaces in project name', add: {}, remove: {},
});
});
test('_handleSaveForReview rules', () => {
// Delete a rule.
element._local['refs/*'].permissions.owner.rules[123].deleted = true;

2
polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.html
View File

@@ -16,7 +16,7 @@ limitations under the License.
-->
<link rel="import" href="../../../behaviors/gr-list-view-behavior/gr-list-view-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../styles/gr-form-styles.html">

2
polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.html
View File

@@ -19,7 +19,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-access-behavior/gr-access-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">

2
polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
View File

@@ -17,7 +17,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-change-table-behavior/gr-change-table-behavior.html">
<link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../styles/gr-change-list-styles.html">

2
polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.html
View File

@@ -16,7 +16,7 @@ limitations under the License.
-->
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-icons/gr-icons.html">

2
polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
View File

@@ -17,7 +17,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-change-table-behavior/gr-change-table-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">

2
polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
View File

@@ -17,7 +17,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-formatted-text/gr-formatted-text.html">

2
polygerrit-ui/app/elements/core/gr-router/gr-router.html
View File

@@ -18,7 +18,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-reporting/gr-reporting.html">

2
polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.html
View File

@@ -15,7 +15,7 @@ See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../shared/gr-autocomplete/gr-autocomplete.html">

2
polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html
View File

@@ -17,7 +17,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior/gr-url-encoding-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">

1
polygerrit-ui/app/test/index.html
View File

@@ -205,6 +205,7 @@ limitations under the License.
'gr-patch-set-behavior/gr-patch-set-behavior_test.html',
'gr-path-list-behavior/gr-path-list-behavior_test.html',
'gr-tooltip-behavior/gr-tooltip-behavior_test.html',
'gr-url-encoding-behavior/gr-url-encoding-behavior_test.html',
'safe-types-behavior/safe-types-behavior_test.html',
];
/* eslint-enable max-len */