scripts aren't protected while loading content

[Alfonso Martínez de Lizarrondo]

Load this in source mode:

<script>alert(0)</script>

when you switch to design view the alert will show, so any script will be executed instead of being protected.

[Artur Formella]

It could be a new feature based on fakeElement:

Plugin to protect custom elements and provide ability to edit content and attibutes in dialog.

Something like that:

protectedElements:[
{
	tagName : "script",			// since 3.0
	className : "cke_script",	// since 3.0
	editable : false			// default in 3.0
},
{
	tagName : "mytag",
	className : "cke_mytag",
	editable : true,			// since 3.x
	check : function( element )	// since 3.x
	{
		return ( element.getAttribute( "yes" ) == "no" );
	}
},
{
	tagName : "gallery",		//for mediawiki
	className : "cke_mv_gallery",
	editable : true
}
]

[Frederico Caldeira Knabben]

Yes arczi we could have a feature for it, but this should not be our focus right now. At this moment we must implement the protection system, just like we have in V2.

[Alfonso Martínez de Lizarrondo]

As Fredck said, the focus should be at least to reproduce the V2 behaviour: I've mentioned only scripts as they are easy to test, but remember that php code, asp... must be also preserved before the browsers parses the html and tries to convert it into DOM nodes breaking it.

[Artur Formella]

JS error in the following case:

<!--
<?xml-stylesheet href="http://www.w3.org/StyleSheets/Core/Steely" type="text/css"?>
<?xml-stylesheet href="http://www.w3.org/StyleSheets/Core/Chocolate" type="text/css"?>
<?xml-stylesheet href="http://www.w3.org/StyleSheets/Core/Midnight" type="text/css"?>
<?xml-stylesheet href="http://www.w3.org/StyleSheets/Core/Modernist" type="text/css"?>
-->

[Artur Formella]

example image. Copy to _source/plugins/protectedsource/images

[Artur Formella]

Idea of this patch: Protect content using regexp before anything and convert it into comments or fake objects.

Usage:

{
	regExp : /regexp/g,
	editable : true/false	//use fakeElement / protectedElement, default false
	name : "mycode",	// used by fakeElement to show in elementpath

	replaceWith : myfunc // custom replace function
	className : "myclass"	// used by fakeElement, default "cke_code"
},

and explanation:

comment.js:  because filter.onComment can return object (not only the comment content) it should use CKEDITOR.htmlParser.fragment(). 

element.js: filter.onElement() can return text node (i.e. content of the fakeElement) so writer.text( element.value ) should be used.

filter.js: New private array: regExp : [].

text.js: new attribute in constructor (protect), default null. If true: don't use  value.replace( spacesRegex, ' ' ) and filter.onText( text )


fakeobjects/plugin.js: 
	In CKEDITOR.editor.prototype.createFakeElement():
		Added new attribute (protectContent), default null. Used in htmlFilterRules.
		typeof realElement can be a string

		Now we can protect element not only in wysiwyg mode:
		var document = this.document || CKEDITOR.document;

	CKEDITOR.editor.prototype.createFakeParserElement
		Added new attibute (protectContent), default null. Added for the compatibility with createFakeElement().

	elements : $ : function( element ):

		if _cke_protect then don't do anything - just return.


htmlDataProcessor/plugin.js: 
	Added protectSource(). It is called before anything to protect HTML using filter._.regExp

Tests (works of course):

<p>
	<script>bhfty<strong>gybf
6t ;()jio)(*&^
%$#sample text

</script> <script  language="JavaScript" a=b><!--bhfty<strong>gybf
6t ;()jio)(*&^
%$#sample text
//-->
</script><style><script  language="JavaScript"><!--bhfty<strong>gybf
6t ;()jio)(*&^
%$#sample text
//-->
</script></style><?<style><script><!--bhfty<strong>gybf
6t ;()jio)(*&^
%$#sample text
//-->
</script></style>?><?
<style><script><!--bhfty<strong>gybf
6t ;()jio)(*&^</script>
%$#sample text
//-->
</style>
?></p>

Known issues:

-Due to method:

<script>
alert("<script><\script>")
</script>

is parsed wrong. Workaround:

<script language="JavaScript">
<!--
alert("<script></script>")
//-->
</script>
or
<script>
alert("<script><\/script>")
</script>

-"code.gif" could be more thoughtful :)

[Alfonso Martínez de Lizarrondo]

Replying to arczi:

Known issues:

-Due to method:

<script>
alert("<script><\script>")
</script>

is parsed wrong. Workaround:

I guess that you mean alert("<script></script>"), but putting that inside a script tag is invalid html, the html parser will close the script at that point (and the javascript parser will say that it's broken) and try to parse the ")</script> as another html block.

[Artur Formella]

New patch in attachment.

Changes:

-simplified implementation and config

-"editable" parameter was removed

-changes from #3441 are used.

[Garry Yao]

The patch works in the expected way, while it's arguable to the html filter for protected other languages for the following reason:

1. The pre-processing of other embeded XML processor instructions, like '<?php>' should not handled by html filter which is responsible for (X)HTML only.The new adding 'regExp' rule for this purpose might get confused with the other rules.
2. It's more reasonable if the restoring of these instruction tags could be defer until the (X)HTML has completed and act as some sort of post-processing with encoded contents inside the comments.
3. Perhaps we should make distinguish between <script>, <style> and instruction tags, <script> is protected by default, <style> not been protected(for good of customized inline style), and <%php%> been protected depend on configuration.

[Frederico Caldeira Knabben]

The problem here, and the thing to keep in mind, is that the protect source feature must leave the original source "perfectly intact". So, we should not use the parser for it... for input processing at least.

This feature is supposed to be much simple. It should be implemented almost entirely in the htmlprocessor plugin:

• On input, the original data is processed, simply running the protection replacement regexes, inserting comments in place of protected text (just like V2).

• On output, a filter for comments would replace them with the relative original string (CDATA).

It should look like V2 in many ways, which is actually a quite simple feature. We don't need a dedicated plugin or too much code for that.

Also, just like stated Garry, some protection is to be done by default, like comments and scripts (just like V2). Others, come from configuration.

The "element" protection idea is interesting, but it would have much few usage. We need it for <script> actually, but we're already doing that internally in the code, so we can come with the proper regex for that.

Also, I don't see the point of having a name for each config entry. So, the config should look like this:

CKEDITOR.config.protectedSource =
    [
        /<%[\s\S]*?%>/g,    // ASP Code
        /(<asp:[^\>]+>[\s|\S]*?<\/asp:[^\>]+>)|(<asp:[^\>]+\/>)/gi,    // ASP.Net Tags
        /<\?[\s\S]*?\?>/g   // PHP Code
    ];

To summarize, we should keep it simple here, not missing the experience we have made with V2.

[Artur Formella]

Ok.

We had problems in v2 with the implementation of protected source. In drupal and mediawiki we need protect new tags ( source, gallery, ref, reference, imagemap,nowiki,... ), templates:

{{}}

Protected source from v2 was simply useless in most cases. My idea was to provide by the way a feature to handle add those requirements. Simple way to add GALLERY tag (maybe in future):

{
	name : 'gallery'	// for elementpatch  and alt for fakeobject
	regExp : /<gallery[^>]*>(.|\n|\r)*?<\/gallery[^>]*>/g 
	className : "cke_mv_gallery",
	editable : true,  // use fakeobject
	editby : "somedialog",
}

We need to protect comments because of

<!-- <? ?> -->

[Artur Formella]

#3407 has been marked as DUP.

[Frederico Caldeira Knabben]

Ok... let's solve things in the proper order, one by one. Otherwise we risk having too many unrelated features to be verified that are just blocking the simplest one, which would fix the issue.

I'm coming with a new patch that will be strictly related to this issue.

[Garry Yao]

The patch is generally good, but I believe we've in some trouble with this case in IE:

<script>alert('')</script>
<p>content</p>

After the <script> has been protected into comments, it's get stripped on after we call IE to help fixing it.See the attached TC for detail.

[Martin Kou]

A similar approach to #3710 should be useful here: before calling IE to fix the HTML via innerHTML, the <script> tag can be converted to a temporary tag containing the original tag's HTML in URL encoded format. After IE has fixed the HTML, revert the transformation.

[Frederico Caldeira Knabben]

This is not a problem with <script> only. It would break any kind of code that gets protected.

This is an old and known issue with IE. If we have a comment at the beginning of the document, it get lost (it happens today without the patch). We have a workaround for it in V2. We need a new ticket for it, but this issue should not block this ticket.

[Garry Yao]

Please include the TC before submitting if you feel NP with it, a dedicate ticket regarding the 'head comment lost' issue will be opened ASA this ticket get fixed.

[Frederico Caldeira Knabben]

Fixed with [3735]. The TC is failing on IE due to #3801.