issue with column title encoding

[Bridge24]

Hi, in our app, customers can create "custom Fields", and they can have any title they want.

Ideally, we always prefer to keep original values in javascript objects without encoding them, and use templates to encode if needed.

I have this very "limit" case, where the title of a columns is "<script>alert(1)</script>".

https://dojo.telerik.com/@foxontherock/imacAYOR/3

There are 3 cases, one with script, one with bold, both with templating, and the last one, bold without template (all default)

I am able to use a groupTemplate and headerTemplate to display them correctly.

But, I tried several methods to make the "columnMenu" appear correctly, I never found a solution.

I tried the "columnMenuInit", but the "alert" appear before the init event.

What I can do for now is set "menu=false" for these columns...

I don't want to "htmlEncode" the title property itself.

Is it possible to set a custom template on the kendoMenu that is displayed to choose the column, or is it possible to set a setting to let the menu encode the text?

For exemple, I can set a title like <b>mytext</b> and the text will appear as bold in the menu!

I think that you forgot to encode the title when the columnmenu is displayed.

The "encode" parameter correctly encode the value, but doesn't affect the title.

Maybe we should get a encodeTitle attribute?

I think that all of this should work correctly without any custom templates.

<note>
You can't say that you don't support it,
because, you did it correctly in the "drop zone" when grouping,
the title is htmlencoded without any need for custom template!!!
just try it with the "bold no template" column and you'll see.
</note>

 

[Georgi]

Hello Daniel,

Generally speaking, a good practice is to store the data in the database as is (not escaped) but return it from the server escaped.

Have you considered returning the data escaped from the server?

Nevertheless, indeed the title is not escaped as it is part of the configuration and therefore the developer sets its value.

Why escaping the tittle field is not an option? The title is displayed correctly if you escape it, is there a reason to avoid it?


Regards,
Georgi
Progress Telerik

[Bridge24]

Hi Georgi. 

As you sais a "good practice" is to store data in the database not escaped.

But, a good practice is also to keep that data unescaped in our javascript properties. 

In our case, the "database" is some other providers like trello and asana that send us custom fields name unescaped. 

And we try to keep them as-is, until they are rendered to screen. 

So, the good place to escape them is in the template. That's why you encode the values of data by default and that's what should be done, or else we'll need to use templates everywhere or escape all text values of all dataset!!!

The same should be applied to titles. When we export in excel, the titles need to be unmodified, excel is not a web page. 

It's uncertain if the titles will go to html, or in other non-html context that don't need to be escaped.

It's sure that we can keep another source for all fields and map between original and grid titles... but if we can keep the grid collection titles as-is, it's preferable, as we'll not need to always encode-decode content...

[Bridge24]

I just add the toolbar with pdf + excel buttons, and as I said, you didi it correctly in some places, including excel fields names.

https://dojo.telerik.com/@foxontherock/imacAYOR/4

I really think that, because you did it well at some places, you just forgot the: 

headertemplate, groupheadertemplate (or groupcolumnheadertempalte?) and grid column menu.

[Georgi]

Hi Daniel,

There is no need to escape field names as they need to be valid Property accessors. The `<` and `>` chars are not allowed in property accessors. Therefore, if your clients provide you with invalid field names, sooner or later you will get a JS error.

Nevertheless, I must agree with you that the content of the title should allow special characters as there might be a case where this is necessary. So as a workaround I can suggest you to override the internal _createMenu method of the column menu.

Below you will find a modified version of the provided sample:

• https://dojo.telerik.com/ItULUzIy

Just on a side note, I have discussed the matter with few colleagues of mine and we are not unanimous. Some think that a title is meant to contain only text and there should be no reason to escape it. Nevertheless, we will further discuss this case and in case we decide to encode the title I will contact you.

I hope this helps.


Regards,
Georgi
Progress Telerik

[Bridge24]

Thank you for this template that solve my columnmenu issue.

About title encoding... it's fun that you have discussions internally about it! 
I still think that you should encode it, everywhere it's needed.... header, groupheader, footer, etc...

And, for the field names, we don't use titles as fields names, we use valid fields names based on custom fields ID, fields like "customField1234" and we have no issues.

In some cases where we can't get invalid names, we set fields name with brackets like: {field: "['abc']"}, with the "from" property of the data schema model.

But the title must accept any valid string, it's not "data-related", it's like a value.
Forget about "html", we may have one user who name its field "Products > Cars" for instance.

Finally, let's try this dojo:

https://dojo.telerik.com/@foxontherock/EFIdeViF/2

Try to group column 1 and 2, and you'll see what's going wrong!  I think that the ">" character should be always visible as >, not half of the time when it "looks like" html!
When grouping, you see the "<b>" string in the group header.

You can show that sample to your friends to help them take their decision bout encoding titles!

Thank you.

[Georgi]

Hello Daniel,

While most of us totally agree with you, this is not as trivial as it looks. We have to investigate this a bit further as some clients might depend on the not escaped title - some people use html in the title instead of the header template. Therefore escaping the title will be a breaking change.

We can consider adding a configuration which when set to true will escape the titles to avoid a breaking change.

Thank you for cooperation.


Regards,
Georgi
Progress Telerik

[Bridge24]

Based on your dojo sample, where you rewrite all the _createMenu function, I tried to make a less-invasive solution, that should be more compatible with all future versions of kendo...

Final solution: https://dojo.telerik.com/@foxontherock/oCacEGIx/2

using that trick that allow the original _createMenu function to stay as-is.

kendo.ui.ColumnMenu.fn._createMenu = (function(){
    var __create = kendo.ui.ColumnMenu.fn._createMenu;
    return function(){
        var that = this;

        that.owner.columns.forEach(function(col){col.title = encode(col.title)});
        var __return =  __create.apply(this, arguments);
        that.owner.columns.forEach(function(col){col.title = decode(col.title)});

        return __return
    };
})();
function encode(_in){return $('<div></div>').text(_in).html()}
function decode(_in){return $('<div></div>').html(_in).text()}

[Georgi]

Hi Daniel,

Indeed encoding the title before the menu is created and then decode it back again is a solution which will not override any future changes in our scripts. I have examined the sample and I can confirm that it looks good. You could omit returning from the custom handler as the _createMenu method does not return anything (I doubt we will change it in future to return a value).

e.g.

return function(){
  var that = this;

  that.owner.columns.forEach(function(col){col.title = encode(col.title)});
  __create.apply(this, arguments);
  that.owner.columns.forEach(function(col){col.title = decode(col.title)});
};

Finally, I have logged an enhancement regarding the titles encoding. You can track it in the following link:

• https://github.com/telerik/kendo-ui-core/issues/5144

Regards,
Georgi
Progress Telerik