views:

717

answers:

2

I'm passing the company name to an onclick event. Some company names have apostrophes in them. I added '.Replace("'", "'")' to the company_name field. This allows the onclick event to fire, but the confirm message displays as "Jane&# 39;s Welding Company".

<a href="#" onclick="return Actionclick('<%= Url.Action("Activate", new {id = item.company_id}) %>', '<%= Html.Encode(item.company1.company_name.Replace("'", "&#39;")) %>');" class="fg-button fg-button-icon-solo ui-state-default ui-corner-all"><span class="ui-icon ui-icon-refresh"></span></a>

<script type="text/javascript">
function Actionclick(url, companyName) 
{
    if (confirm('This action will activate this company\'s primary company ('+companyName+') and all of its other subsidiaries.  Continue?')) 
    {
        location.href = url;
    };
};

EDIT The confirm message shows the &# 39; in the message rather than the '. When I typed it out here, it replaced the &# 39; with a '. Added spaces so that wouldn't happen. I want to know the best way to pass it to my onclick event and also properly display it in the message without doing multiple replaces (if there is a better way).

+3  A: 

There are two options as I see it.

  1. If you wrap the parameters in quotes (") instead of apostrophes/single quotes (') then you shouldn't need to escape it at all. HTML encoding will take care of encoding any quotes (if they are in the string) and the apostrophe's won't be a problem. Though, as the javascript is already wrapped in quotes, you will need to backslash escape your quotes. eg:

    onclick="return Actionclick(\"<%= Url.Action("Activate", new {id = item.company_id}) %>\", \"<%= Html.Encode(item.company1.company_name) %>\");"

  2. Backslash escape the company name as it's only the final javascript string that needs the apostrophe escaped, not the HTML. eg:

    onclick="return Actionclick('<%= Url.Action("Activate", new {id = item.company_id}) %>', '<%= Html.Encode(item.company1.company_name.Replace("'", "\\'")) %>');"

Brenton Alker
I tried #2 and it wouldn't compile. Kept warning me that I was missing a )I'll try #1.
RememberME
I missed the escaping of the backslash itself in #2. Edited.
Brenton Alker
I had the second backslash, but I went back to a previous version and then tried adding the .Replace("'", "\\'") and it worked. I looked and looked and I don't know what the issue was, but I must have introduced something somewhere else in the line. Works now. Thanks.
RememberME
Thank you so much!!! Just helped me with my problem!!!
Jenski
A: 

You've got a JavaScript string literal inside an HTML attribute value.

So you would need to first JS-encode the value (replacing the ' with \' and \ with \\), then HTML-encode. Currently you are HTML-encoding the ' (which would be ineffective, since the browser would decode it back to an apostrophe before the JS engine saw it)... and then HTML-encoding it again, leaving it literally meaning &#39;.

Use a JSON encoder to turn a string (or any other value type) into a JavaScript literal.

However. Writing JavaScript in a string utterly sucks. Keeping track of multiple layers of escaping isn't something the mind is good at. So don't do it. Avoid inline event handler attributes at all times. Instead, use static script and assign handlers from JavaScript itself, using unobtrusive scripting.

<a class="dangerous fg-button fg-button-icon-solo ui-state-default ui-corner-all"
    href="<%= Server.HTMLEncode(Url.Action("Activate", new {id = item.company_id})) %>"
    title="This action will activate this company's primary company (<%= Server.HTMLEncode(companyName) %>) and all of its other subsidiaries."
>
    <span class="ui-icon ui-icon-refresh"></span>
</a>

(I'll use jQuery since you have it in your tags:)

<script type="text/javascript">
    $('.dangerous').click(function() {
        return confirm(this.title+' Continue?');
    });
</script>

However note that this is an abuse of <a>. Actions that make an active change to something should never be sent, or be allowed to be received, as a GET request. You should instead use a button that submits a POST request (either directly as a form, or via AJAX). (You should also consider using ASP.NET's built-in controls instead of templating the values in, to avoid having to call HTMLEncode quite so much.)

See this classic WTF for one way in which this can bite you.

bobince