Fix ids of permission checkboxes for shares

The ids of permission checkboxes for shares were generated using the
"shareWith" field of the share. The "shareWith" field can contain spaces
(as spaces are allowed, for example, in user or circle names), so this
could cause the id attribute of the HTML element to contain spaces too,
which is forbidden by the HTML specification.

It is not just a "formal" issue, though; when the list was rendered, if
the id contained a space the selector to get the checkbox element was
wrong (as it ended being something like
"#canEdit-view1-name with spaces") and thus the initial state of the
checkbox was not properly set.

Besides that, "shareWith" can contain too single quotes, which would
even cause the jQuery selector to abort the search and leave the UI in
an invalid state.

Instead of adding more cases to the regular expression to escape special
characters and apply it too when the ids are created now the ids of
permission checkboxes for shares are based on the "shareId" field
instead of on "shareWith", as "shareId" is expected to always contain
compatible characters.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This commit is contained in:
Daniel Calviño Sánchez 2018-04-18 14:07:04 +02:00
parent b2a87f84c4
commit 1466586033
2 changed files with 41 additions and 11 deletions

View File

@ -30,8 +30,8 @@
'<span class="sharingOptionsGroup">' + '<span class="sharingOptionsGroup">' +
'{{#if editPermissionPossible}}' + '{{#if editPermissionPossible}}' +
'<span class="shareOption">' + '<span class="shareOption">' +
'<input id="canEdit-{{cid}}-{{shareWith}}" type="checkbox" name="edit" class="permissions checkbox" />' + '<input id="canEdit-{{cid}}-{{shareId}}" type="checkbox" name="edit" class="permissions checkbox" />' +
'<label for="canEdit-{{cid}}-{{shareWith}}">{{canEditLabel}}</label>' + '<label for="canEdit-{{cid}}-{{shareId}}">{{canEditLabel}}</label>' +
'</span>' + '</span>' +
'{{/if}}' + '{{/if}}' +
'<a href="#"><span class="icon icon-more"></span></a>' + '<a href="#"><span class="icon icon-more"></span></a>' +
@ -58,8 +58,8 @@
'{{#if isResharingAllowed}} {{#if sharePermissionPossible}} {{#unless isMailShare}}' + '{{#if isResharingAllowed}} {{#if sharePermissionPossible}} {{#unless isMailShare}}' +
'<li>' + '<li>' +
'<span class="shareOption menuitem">' + '<span class="shareOption menuitem">' +
'<input id="canShare-{{cid}}-{{shareWith}}" type="checkbox" name="share" class="permissions checkbox" {{#if hasSharePermission}}checked="checked"{{/if}} data-permissions="{{sharePermission}}" />' + '<input id="canShare-{{cid}}-{{shareId}}" type="checkbox" name="share" class="permissions checkbox" {{#if hasSharePermission}}checked="checked"{{/if}} data-permissions="{{sharePermission}}" />' +
'<label for="canShare-{{cid}}-{{shareWith}}">{{canShareLabel}}</label>' + '<label for="canShare-{{cid}}-{{shareId}}">{{canShareLabel}}</label>' +
'</span>' + '</span>' +
'</li>' + '</li>' +
'{{/unless}} {{/if}} {{/if}}' + '{{/unless}} {{/if}} {{/if}}' +
@ -67,24 +67,24 @@
'{{#if createPermissionPossible}}{{#unless isMailShare}}' + '{{#if createPermissionPossible}}{{#unless isMailShare}}' +
'<li>' + '<li>' +
'<span class="shareOption menuitem">' + '<span class="shareOption menuitem">' +
'<input id="canCreate-{{cid}}-{{shareWith}}" type="checkbox" name="create" class="permissions checkbox" {{#if hasCreatePermission}}checked="checked"{{/if}} data-permissions="{{createPermission}}"/>' + '<input id="canCreate-{{cid}}-{{shareId}}" type="checkbox" name="create" class="permissions checkbox" {{#if hasCreatePermission}}checked="checked"{{/if}} data-permissions="{{createPermission}}"/>' +
'<label for="canCreate-{{cid}}-{{shareWith}}">{{createPermissionLabel}}</label>' + '<label for="canCreate-{{cid}}-{{shareId}}">{{createPermissionLabel}}</label>' +
'</span>' + '</span>' +
'</li>' + '</li>' +
'{{/unless}}{{/if}}' + '{{/unless}}{{/if}}' +
'{{#if updatePermissionPossible}}{{#unless isMailShare}}' + '{{#if updatePermissionPossible}}{{#unless isMailShare}}' +
'<li>' + '<li>' +
'<span class="shareOption menuitem">' + '<span class="shareOption menuitem">' +
'<input id="canUpdate-{{cid}}-{{shareWith}}" type="checkbox" name="update" class="permissions checkbox" {{#if hasUpdatePermission}}checked="checked"{{/if}} data-permissions="{{updatePermission}}"/>' + '<input id="canUpdate-{{cid}}-{{shareId}}" type="checkbox" name="update" class="permissions checkbox" {{#if hasUpdatePermission}}checked="checked"{{/if}} data-permissions="{{updatePermission}}"/>' +
'<label for="canUpdate-{{cid}}-{{shareWith}}">{{updatePermissionLabel}}</label>' + '<label for="canUpdate-{{cid}}-{{shareId}}">{{updatePermissionLabel}}</label>' +
'</span>' + '</span>' +
'</li>' + '</li>' +
'{{/unless}}{{/if}}' + '{{/unless}}{{/if}}' +
'{{#if deletePermissionPossible}}{{#unless isMailShare}}' + '{{#if deletePermissionPossible}}{{#unless isMailShare}}' +
'<li>' + '<li>' +
'<span class="shareOption menuitem">' + '<span class="shareOption menuitem">' +
'<input id="canDelete-{{cid}}-{{shareWith}}" type="checkbox" name="delete" class="permissions checkbox" {{#if hasDeletePermission}}checked="checked"{{/if}} data-permissions="{{deletePermission}}"/>' + '<input id="canDelete-{{cid}}-{{shareId}}" type="checkbox" name="delete" class="permissions checkbox" {{#if hasDeletePermission}}checked="checked"{{/if}} data-permissions="{{deletePermission}}"/>' +
'<label for="canDelete-{{cid}}-{{shareWith}}">{{deletePermissionLabel}}</label>' + '<label for="canDelete-{{cid}}-{{shareId}}">{{deletePermissionLabel}}</label>' +
'</span>' + '</span>' +
'</li>' + '</li>' +
'{{/unless}}{{/if}}' + '{{/unless}}{{/if}}' +
@ -383,7 +383,7 @@
var _this = this; var _this = this;
this.getShareeList().forEach(function(sharee) { this.getShareeList().forEach(function(sharee) {
var checkBoxId = 'canEdit-' + _this.cid + '-' + sharee.shareWith; var checkBoxId = 'canEdit-' + _this.cid + '-' + sharee.shareId;
checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@|\/)/g, "\\$1"); checkBoxId = '#' + checkBoxId.replace( /(:|\.|\[|\]|,|=|@|\/)/g, "\\$1");
var $edit = _this.$(checkBoxId); var $edit = _this.$(checkBoxId);
if($edit.length === 1) { if($edit.length === 1) {

View File

@ -105,6 +105,21 @@ describe('OC.Share.ShareDialogShareeListView', function () {
expect(listView.$el.find("input[name='edit']").is(':indeterminate')).toEqual(true); expect(listView.$el.find("input[name='edit']").is(':indeterminate')).toEqual(true);
}); });
it('marks edit box as indeterminate when only some permissions are given for sharee with special characters', function () {
shareModel.set('shares', [{
id: 100,
item_source: 123,
permissions: 1 | OC.PERMISSION_UPDATE,
share_type: OC.Share.SHARE_TYPE_USER,
share_with: 'user _.@-\'',
share_with_displayname: 'User One',
itemType: 'folder'
}]);
shareModel.set('itemType', 'folder');
listView.render();
expect(listView.$el.find("input[name='edit']").is(':indeterminate')).toEqual(true);
});
it('Checks edit box when all permissions are given', function () { it('Checks edit box when all permissions are given', function () {
shareModel.set('shares', [{ shareModel.set('shares', [{
id: 100, id: 100,
@ -119,6 +134,21 @@ describe('OC.Share.ShareDialogShareeListView', function () {
listView.render(); listView.render();
expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true); expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true);
}); });
it('Checks edit box when all permissions are given for sharee with special characters', function () {
shareModel.set('shares', [{
id: 100,
item_source: 123,
permissions: 1 | OC.PERMISSION_CREATE | OC.PERMISSION_UPDATE | OC.PERMISSION_DELETE,
share_type: OC.Share.SHARE_TYPE_USER,
share_with: 'user _.@-\'',
share_with_displayname: 'User One',
itemType: 'folder'
}]);
shareModel.set('itemType', 'folder');
listView.render();
expect(listView.$el.find("input[name='edit']").is(':checked')).toEqual(true);
});
}); });
describe('Manages checkbox events correctly', function () { describe('Manages checkbox events correctly', function () {
it('Checks cruds boxes when edit box checked', function () { it('Checks cruds boxes when edit box checked', function () {