Opened 4 years ago

Last modified 4 years ago

#12895 confirmed Bug

allowedContent for form elements can be improved

Reported by: jhub Owned by:
Priority: Normal Milestone:
Component: General Version: 4.1
Keywords: Cc:

Description

In the plugin.js for the "Forms" plugin, the code explicitly defines different allowed contents for different form elements:

allowedContent = {
	checkbox: 'input[type,name,checked]',
	radio: 'input[type,name,checked]',
	textfield: 'input[type,name,value,size,maxlength]',
	textarea: 'textarea[cols,rows,name]',
	select: 'select[name,size,multiple]; option[value,selected]',
	button: 'input[type,name,value]',
	form: 'form[action,name,id,enctype,target,method]',
	hiddenfield: 'input[type,name,value]',
	imagebutton: 'input[type,alt,src]{width,height,border,border-width,border-style,margin,float}'
},

but since "checkbox", "radio", "textfield", "button", "hiddenfield" and "imagebutton" all define their allowed content for the same tag (with the name <input>), what we end up with is actually that the combination of all the allowed contents for all these form elements is allowed for all of them.

For example, even though "checkbox" explicitly lists that the allowed content is only "input[type,name,checked]", it is still possible to add for example the "src" attribute by hand to an <input> tag with type="checkbox" (in source code mode) and it will not be removed, even though it was initially not listed as allowed content.

I guess it is not removed because the "imagebutton" defines "src" as a valid attribute for the <input> tag, and thus *all* <input> tags now allow the "src" attribute, even tags with type="checkbox" where the "src" attribute does not make any sense.

Proposed fix: For the allowed content, instead of the string notation, use the object notation with a match function, like this:

allowedContent = {
	checkbox: {
		input: {
			match: function (element)  { return element.attributes.type == "checkbox"; },
			attributes: "type,name,checked"
		}
	},
	radio: {
		input: {
			match: function (element)  { return element.attributes.type == "radio"; },
			attributes: "type,name,checked"
		}
	},
	textfield: {
		input: {
			match: function (element)  { return element.attributes.type == "text" || element.attributes.type == "email" || element.attributes.type == "password" || element.attributes.type == "search" || element.attributes.type == "tel" || element.attributes.type == "url"; },
			attributes: "type,name,value,size,maxlength"
		}
	},
	textarea: 'textarea[cols,rows,name]',
	select: 'select[name,size,multiple]; option[value,selected]',
	button: {
		input: {
			match: function (element)  { return element.attributes.type == "button" || element.attributes.type == "submit" || element.attributes.type == "reset"; }, // TODO
			attributes: "type,name,value"
		}
	},
	form: 'form[action,name,id,enctype,target,method]',
	hiddenfield: {
		input: {
			match: function (element)  { return element.attributes.type == "hidden"; },
			attributes: "type,name,value"
		}
	},
	imagebutton: {
		input: {
			match: function (element)  { return element.attributes.type == "image"; },
			attributes: "type,alt,src",
			styles: "width,height,border,border-width,border-style,margin,float"
		}
	}
}

The proposal above should be equivalent to the original code, only the added "match" functions should make sure that for each type-variant of the <input> tag, only the actually defined allowed content is applied.

Of course you then may want to revise these individual allowed content definitions, for example for the imagebutton, you may want to allow "name" and "value" too?

Change History (3)

comment:1 Changed 4 years ago by jhub

P.S. My proposal of above was only the bare bones code. I should probably have mentioned that you also need to modify the "requiredContent", so that it says checkbox: 'input[type]' instead of only checkbox: 'input' (and the same for the 5 other occurences of "input" in the requiredContent). And the six "match" functions in my proposal then all need to include return element.attributes.type == "cke-test" || REST_OF_CONDITION"

comment:2 Changed 4 years ago by Jakub Ś

Status: newconfirmed
Version: 4.4.54.3

it is still possible to add for example the "src" attribute by hand to an <input> tag with type="checkbox" (in source code mode) and it will not be removed,

It is enough to confirm this issue.

comment:3 Changed 4 years ago by Piotrek Koszuliński

Version: 4.34.1
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy