Using single quotes in an HTML attribute?

JavaScript
1

I have a set of items stored in a database, each with their own unique name.

On one of my HTML pages, I create a list of buttons that display the name of each item, and when they are clicked, I call a function that passes the name of the item as a parameter.

<div class="large professional-button" onclick="doStuff("itemName", "param2")">itemName</div>

The issue is, these item names are user-generated, so they can be whatever a person wants. This means they have the potential to have single quotes in them, for example: name'with'quotes. I have found that my HTML construction runs into an error when it tries to construct the buttons for any items with quotes in the name. They end up looking something like this:

<div class="large professional-button" onclick="doStuff("name" with'quotes",="" "param2")'="">name'with'quotes</div>

As soon as it hits the first single quotes in the name, it seems to turn them into a space and double-quotes ( ") and then everything else after that becomes a garbled mess. Any double quotes seem to get converted to &quot;.

What should I do so that I can allow single quotes in an item name, and then use that name in an HTML attribute later? I already am using encodeuricomponent() when taking the names in and out of the database, but that function doesn’t affect single quotes.

2

Maybe use the escape character ( / ) to prevent JavaScript from interpreting a quote as the end of the string, or the same in PHP somewhere in your syntax. Sorry that is the best I can do.

3

Unfortunately, seems like that doesn’t work. Here’s when I use decodedName.replace(/'/g, "\\'"); to add escape characters before all of the quotes and log the string being inserted into the HTML:

image

4

Please supply more details about the database.

If and only if the database is PHP driven then there are numerous PHP functions available to convert the source before storing. On retrieval the data should render without any problems.

PHP functions to investigate:

Htmlspecialchars(…)
htmlentities(…)
strip_tags(…)

Reminds me of…

https://www.google.com/url?q=https://en.wikipedia.org/wiki/Garbage_in,_garbage_out&sa=U&ved=2ahUKEwiusbqFy5vpAhXCbSsKHS9eD1kQFjAAegQICBAB&usg=AOvVaw023GvfHssxCw7xmBxOiMft

1 Like
5

Are you saying you’re generating JS from user content? This should by all means be avoided; for instance, consider one item would be

"); doSomethingEvil(); void ("

… then you would have malicious code injected in your click handler:

<div onclick='doStuff(""); doSomethingEvil(); void ("", "param2")'>itemName</div>

(Relevant XKCD:)

4 Likes
6

Hi @John_Betong! Yes, the database is completely PHP driven. Apologies for not making that explicit.

Nothing happening during the storage is an issue. I am encoding the user-given strings as well as using filter_var in my PHP code to sanitize the input. I decode everything when it comes out of the database and end up with the exact same string that the user inputted. The problem is that when I try to take names that involve single quotes (') out of the database and put them into the HTML, the HTML seems to break. It doesn’t appear to have anything to do with the storage/retrieval aspect of getting the names.

Sure. Are you implying the solution is to just bar users from inputting the single quote symbol?

@m3g4p0p I sanitize the user input when I take it in and out of the database (see above in this response) to avoid a Little Bobby Tables situation. When it comes to inserting the user input into the HTML attribute, though, what is the best practice for ensuring safety and preventing injection? I have been doing some simple sanitizing like converting spaces to hyphens, but what is the ideal way to keep it clean? Using encodeURIComponent? (though even if I use encodeURIComponent, it will not encode single quotes, so my original problem will still remain.)

7

I have just experienced a similar problem and used the following PHP function to replace the single quote and it works OK:

    # rep[lace apostrophe with ASCII character
    $title = str_replace("'", "&#39;", $title);

Google “html ascii character table”

Garbage IN:

NO! The idea is to check and modify user’s input to remove or replace any characters which may cause problems before storing to the database table.

Garbage OUT:

All database data has been checked, cleaned and ready for usage using PHP str_replace(…);

https://www.php.net/manual/en/function.str-replace.php.

Enhance this PHP function to suit your requirements

//======================================
function fnClean(string $tmp)
:string
{
  // good and bad can be an array of items
  $bad  = "'"; 
  $good = '"&#39;"';
  $tmp  = str_replace( $bad, $good, $tmp);

  return $tmp;
}
1 Like
8

Don’t generate your JS dynamically at all, especially if user content is involved; and even if not, this gets hard to read and maintain pretty quickly. I assume you’re currently doing something like this?

echo "<div onclick='doStuff(\"" . $item . "\")'>...</div>";

Instead, you might store the values as data attributes:

echo "<div data-item='" . $item . "' onclick='doStuff(this.dataset.item)'>...</div>";

This way you can be sure that the the value of $item won’t get evaluated but always treated as a string by your JS. And while at it, you might also consider using addEventListener() instead to keep your PHP and JS logic separate:

echo "<div data-item='" . $item . "' class='js-do-stuff'>...</div>";

And then in your JS (at the bottom of the page, before the closing </body> tag):

function doStuff (item) {
  // ...
}

var target = document.querySelector('.js-do-stuff')
target.addEventListener('click', function () {
  doStuff(this.dataset.item)
})

You might actually avoid the wrapper function here though:

function doStuff () {
  var item = this.dataset.item
  // ...
}

var target = document.querySelector('.js-do-stuff')
target.addEventListener('click', doStuff)

And if you have more than just one target:

function doStuff () {
  var item = this.dataset.item
  // ...
}

var targets = document.querySelectorAll('.js-do-stuff')
targets.forEach(function (target) {
  target.addEventListener('click', doStuff)
})
4 Likes
9

What you do it stop using HTML inline event handlers.

There are much better ways to deal with things when using addEventListener instead.
With the params, you can store that in a data attribute, and use dataset to retrieve it.

For example:

<div class="large professional-button" data-param="param2">itemName</div>

function buttonClickHandler(evt) {
    const button = evt.target;
    const itemName = button.textContent;
    const param = button.dataset.param;
    doStuff(itemName, param);
}

const largeButtons = document.querySelectorAll(".large professional-button");
largeButtons.forEach(function (button) {
    button.addEventListener("click", buttonClickHandler);
});
10

Oh, that’s a great idea! Encoding the single quotes as HTML entities before putting them on the page, I should have thought of that! It works like a charm, thank you @John_Betong!

Oh, nifty, I did not know about data attributes before! Besides just looking cleaner, does the extra security come from data attributes ensuring that the user-input is parsed as a String and not as runn-able code?

Great, alright. I was never given formal training on JavaScript/HTML structuring practices (I was self-taught in that area), so I didn’t know that it is better practice to keep all function calls in external Event Listeners instead of inline ones. Are there any other small best-practice formatting/structuring conventions I should be aware of?

I’ve officially refactored to move all my listeners into external functions and put all of the information they need into data attributes. Thanks for the help everyone, this was very valuable to me!

2 Likes
11

Yes, exactly.

12

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.