Add icon support to Selectlist #626
Conversation
| @@ -62,7 +62,8 @@ | |||
| this.$selectedItem = $selectedItem = $item; | |||
|
|
|||
| this.$hiddenField.val(this.$selectedItem.attr('data-value')); | |||
| this.$label.text(this.$selectedItem.text()); | |||
| // Shallow copy | |||
| this.$label.html( $(this.$selectedItem.children()[0]).html() ); | |||
There was a problem hiding this comment.
is this just asking for XSS?
1aa7876 to
23d2673
Compare
|
Using a chainsaw instead of a scalpel can get messy... |
|
I don't think there's a huge XSS issue here. It's utilizing content already defined on the page by the developer, with no input from the user. Additionally, the value that is submitted via form post is not the markup itself, but a separate value altogether. If it were combobox this could be a big problem but with selectlist I believe we're ok. |
|
True, but putting any sort of HTML into the page without checking it first is poor form. yes it should be done by the developer implementing it as well, but I don't think we should provide an easy way for it to happen if they mess up |
|
@vernak2539 Would you be more comfortable with: |
|
@vernak2539 would it be ok if it only uses already-existing DOM nodes and merely "moves" them? |
|
I would think @futuremint's suggestion would be a good path since we wouldn't be "adding/updating" anything. I'll add this is a weird subject and requires the developer to do a good amount of work implementing as @kevinparkerson said |
|
I'm not sure moving would work because there are scenarios where the content would need to be displayed in multiple places at once. For instance, when the selectlist is expanded the content would be showing both within the button and within the menu item. I do share your concern for XSS vulnerabilities, but in this case, I'm not sure there is one. 1) the content that is being cloned is already rendered and in the DOM 2) there is no direct user input here 3) we're exposing no API/method on the control that would allow you to inject XSS |
|
This code just takes HTML that is already on the page and puts it in another location. XSS would have occurred before this code even touched anything, meaning this code has nothing to do with an XSS attack. |
23d2673 to
9229447
Compare
|
sweet, no worries then. still getting my brain in the mix this week |
Add icon support to Selectlist
No description provided.