2

I have some code that displays the list of article titles, their short descriptions and the authors' name in the following format:

Title (authors' name)
description

Author's names and descriptions are not relevant here because they always display correctly. Most of the titles display correctly too, here are some made up examples:

The Single Most Important Thing You Need To Know About Banking (authors' name) - displays correctly

How Power & Utilities Made Me A Better Salesperson (authors' name) - displays correctly

PG&E And The Chuck Norris Effect (authors' name) - displays incorrectly in the following way: &E And The Chuck Norris Effect (authors' name)

There is a problem only with displaying this single example. That is why I focused on '&' symbol. But '&' doesn't seem to be the problem in other titles where '&' has a space before and after it.

My code to fix this issue is like this but it doesn't affect the output in any way...

// this is not my code
offerRep += '<a _urltype="11" href="' + q.docUrl + '" alt="' + q.documentTitle +'" >' + q.documentTitle + ' "' + fullSubTitle + '"' + ' (' + analystName[0] + ')</a>';

// this is my code 
if (q.documentTitle.indexOf('&') > -1) {
  q.documentTitle.replace(/'&'/g, '&amp;');
} else {
  return q.documentTitle;
}
Mario Boss
  • 1,784
  • 3
  • 20
  • 43
Justyna
  • 29
  • 3
  • 1
    The problem lies in the "not my code" part. Is it possible to change it? – Salman A Sep 27 '19 at 10:24
  • Yes, deffinitelly. But there is more of this code, which is not mine. If you have any hints for me I would be very happy. I am stuck here... – Justyna Sep 27 '19 at 10:33
  • 1
    I would suggest something like `var a = document.createElement('a'); a.href='//example.com'; a.appendChild(document.createTextNode('ad')); console.log(a.outerHTML);`. If you have included jQuery in your project then it would require fewer lines of code but still more than the "not my code part". – Salman A Sep 27 '19 at 10:36

1 Answers1

0

First problem - which maybe because you only posted part of the code (*) - this line:

q.documentTitle.replace(/'&'/g, '&amp;');

never does anything, because replace returns a new string which you are not returning or using in any way.

Then the regular expression /'&'/ means you are looking for a apostrophe followed by a ampersand followed by an apostrophe. You just need /&/.

Using if with indexOf isn't really useful here. If replace doesn't find the regular expression it will simply do nothing so drop the if.

Generally it's a very bad idea just to blindly build HTML with unknown external content like this and it's not only the & that needs escaping. Have a look at: Can I escape html special chars in javascript?

Use a general function and escape ALL external data you want to insert into the HTML:

function escapeHtml(unsafe) {
  return unsafe
     .replace(/&/g, "&amp;")
     .replace(/</g, "&lt;")
     .replace(/>/g, "&gt;")
     .replace(/"/g, "&quot;")
     .replace(/'/g, "&#039;");
 }


offerRep += '<a _urltype="11" href="' + escapeHtml(q.docUrl) + 
            '" alt="' + escapeHtml(q.documentTitle) +'" >' + 
            escapeHtml(q.documentTitle) + ' "' + escapeHtml(fullSubTitle) + '"' + 
            ' (' + escapeHtml(analystName[0]) + ')</a>';

Salman A's suggestion in the comments is also a good way, because you don't need to "remember" how and what to escape.

A safer, better and more modern way would be to use a template engine. For example Handlebars.js (but there are many many more):

var linkTemplate = Handlebars.compile('<a _urltype="11" href="{{url}}" alt="{{title}}">{{title}} "{{subtitle}}" ({{analystname}})</a>');

var linkHtml = linkTemplate({
                 url: q.docUrl,
                 title: q.documentTitle,
                 subtitle: fullSubTitle,
                 analystname: analystName[0]
               });

offerRep += linkHtml;

(*) When asking questions with code ALWAYS post the code with useful context, never just a few lines that don't work on their own.

RoToRa
  • 37,635
  • 12
  • 69
  • 105