Issue Codes with Apostrophes not working

ajperezcrespo wrote on Wednesday, November 28, 2012:

Hi,

  In issues any Code with an apostrophe in the text  are listed when seached but not selectable.  Verified in both 4.1.0(13) , 4.1.1(7) and on the Demo site.  Try Bell’s in the search.

Works in the Fee Sheet just fine.

Thanks

bradymiller wrote on Thursday, November 29, 2012:

Hi,

Here’s a fix for this (I think; it works, but unsure whether it is proper coding):
http://github.com/bradymiller/openemr/commit/b0dd348229a3d0e1e5af3847a6f42ed22c169473

Can other developers please look at this strategy to properly escape literals (found it via google). It seems odd but the following seems to escape javascript literals properly:
attr(addslashes($string))

Any thoughts here? Should we change our developer guide (now only recommend addslashes($string) for javascript literals)?

-brady
OpenEMR

yehster wrote on Thursday, November 29, 2012:

I don’t think it’s correct to both addslashes and use htmlentities-ENT_QUOTES(attr)

http://www.w3.org/TR/html401/intro/sgmltut.html#attributes
There is no mention of backslash being a valid escape mechanism.

addslashes is primarily for database operations, not HTML.

yehster wrote on Thursday, November 29, 2012:

BTW.  The backslashes which appear in this block of code are escaping the quotes for the PHP parser, not as part of an html attribute.

bradymiller wrote on Friday, November 30, 2012:

Hi,

The thing that strikes me with this method is that it works. Otherwise these type of literals will always fail with either single-quote or double-quote (I have never been able to support both in these embedded onclick javascript literals).
Here are the links describing the method:
http://www.zedwood.com/article/84/how-to-properly-escape-inline-javascript
http://www.webmasterworld.com/forum21/6373.htm
(Note that running it through the attr() basically converts the quotes to the codes that are done manually in the above links)

-brady
OpenEMR

yehster wrote on Friday, November 30, 2012:

I’ve tested just attr for single quote, and it works for this case.

Also your second link confirms what I said about backslash not being a valid escaping method.

Escaping as htmlentities really should be sufficient. 

ok, the \ excape doesn’t work. That is the first thing I tried.

bradymiller wrote on Friday, November 30, 2012:

Hi,

But it won’t work for the double quote. In the Search field of popup, enter pneumonitis and then select:
“Ventilation” pneumonitis
(I am assuming you have imported the ICD9 codes)

It only works with attr(addslashes())

-brady
OpenEMR

yehster wrote on Friday, November 30, 2012:

Ok… It makes sense that both addslashes and attr are needed for this to work.  Addslashes is escaping for javascript, while attr is escaping html.
With just addslashes, the html generated is as follows:

<a href='' onclick='return selcode("ICD9", "351.0", "", "Bell\'s palsy")'>Bell's palsy</a>

The single quote in Bell’s closes the onclick attribute string prematurely and you have invalid html.
The slash doesn’t fix it because the issue is with the HTML, and backslash does not do anything in html.  The backslash does escape the single quote for javascript, which is unneeded since the javascript literal uses double quotes, but it doesn’t hurt anything.

For double quotes.  With just attr, the browser parses the HTML like this:

<a onclick="return selcode("ICD9", "495.7", "", ""ventilation" pneumonitis")" href="">

The javascript function call to selcode is invalid because of the double quotes make the 4th parameter incorrectly formed.  The slashes are needed to escape double quote within the string for javascript.

Interestingly, doing the escape sequence in the other order

addslashes(attr($itertext))

doesn’t work because if the quotes get escaped for html first, the slashes don’t get added for the javascript.

Anyway.  Javascript string literals do need to be escaped with addslashes.  HTML attributes need to be escaped with attr.  Since in this case onclick is an HTML attribute which contains javascript literals it does need both.

bradymiller wrote on Friday, November 30, 2012:

hi,
Thanks for the explanation. Makes sense. I’ll commit the fix then,
-brady
OpenEMR

yehster wrote on Friday, November 30, 2012:

https://github.com/yehster/openemr/blob/85e6cce3ec37ad0fea682446f266f22ab9506c97/Tests/SpecialCharsTest.php

I built a set of example which cover a variety of usage scenarios and illustrate what works and doesn’t, and differences in behaviors.

This issue scenario corresponds to the case “On Click as Attribute”.

pfwilliams wrote on Tuesday, December 17, 2013:

When we converted from 4.1.1.14 to 4.1.2.4, two unexpected events took place. First, all user permissions were lost, so we redid the ACL’s manually. Second, our clickoptions list quit working. I didn’t see any documentation on it, but it appeared the list was disabled in favor of entering the data into the list_options table. So, I threw a script together to load the *_issues_list sections of the list_options table. It seemed to work fine. This really isn’t related to the conversion process just described, but we’re finding that any entries containing apostrophes in the list_options table, like “Alzheimer’s” or “Grave’s Disease” are not displaying correctly. The apostrophes have been replaced with “&#039s”. I’m told our dictation entries are now exhibiting the same apostrophe issue. We have the XAMPP version installed and I wonder is this behavior the same for everyone? There is a fix in the works?
Thank you.

fsgl wrote on Tuesday, December 17, 2013:

In 4.1.0 and 4.1.1, I was unable to add the apostrophe with entries in Issues.

After upgrading to 4.1.2, editing of Issues was not possible. The entry had to be deleted and re-entered. On a fluke, I ran the upgrade script again and the Issues problem was resolved. (XAMPP-OpenEMR package on a Windows 7 machine.) It’s a mystery to everyone why this is the case. CVerk had to run the upgrade twice as well.

I have noticed that &#039s appears very fleetingly after I hit the apostrophe key but it is quickly replaced by the correct symbol.

pfwilliams wrote on Thursday, December 26, 2013:

Somewhere in tinkering with the Dictation entries (I did some direct database updates using MySQL Workbench, and also some edits from within OpenEMR) something went “click” and strings like “patient’s” which occur frequently within dictations started to display correctly (even in dictations I had not toyed with). Entries in the list_options table are being obstinate. Despite extensive ‘tinkering’ and rerunning the SQL upgrade script, they continue to display apostrophes as an escaped ASCII representation, rather than the character itself. If you add one of those problem entries to a patient, it will propagate into the lists table and still display improperly, even though it looks fine (shows apostrophe) when viewed in Workbench or MyPHPAdmin. It’s odd that this is not an issue for most, and just as odd are the “fixes” that have worked for some having the problem.
It’s like there’s a flag or switch at the table or column level, or on the variable, some setting somewhere not being consistently set, and it results in an auto-escaping of the apostrophe and then hard-coded in the source are commands that result in the character being double-escaped.

fsgl wrote on Friday, December 27, 2013:

It probably is an issue for other users; but because it is not a show stopper and because there is not enough time to post; they put up with it.

cmswest wrote on Friday, December 27, 2013:

here’s a fix:

change the attrs to addslashes

note, you only have to change the option_id one for some reason as the javascript is loading that as the “title” from list_options

also need to fix so when you delete an issue the patient’s list should be refreshed to show the updated current list

also Kevin’s tutorial link above is not functioning