Bug 23093 - Error during upgrade of OpacNavRight preference to Koha news
Summary: Error during upgrade of OpacNavRight preference to Koha news
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Nick Clemens
URL:
Keywords:
Depends on: 22318
Blocks:
  Show dependency treegraph
 
Reported: 2019-06-11 12:39 UTC by Owen Leonard
Modified: 2020-06-04 20:32 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.01


Attachments
Bug 23093: (bug 22318 follow-up) Use placeholders in query moving OpacNavRight (1.62 KB, patch)
2019-06-11 14:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23093: (bug 22318 follow-up) Use placeholders in query moving OpacNavRight (1.67 KB, patch)
2019-06-11 14:59 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 23093: (bug 22318 follow-up) Use placeholders in query moving OpacNavRight (1.72 KB, patch)
2019-06-12 13:55 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2019-06-11 12:39:41 UTC
Copied from Bug 22318:

Moving the contents of OpacNavRight to opac_news fails due to improper (no?) escaping:

DBD::mysql::db do failed: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'http':'https';if(!d.getElementById(id)){js=d.createElement(s);js.id=id;js.src=p+' at line 1 [for Statement "INSERT INTO opac_news (branchcode, lang, title, content ) VALUES (NULL, 'OpacNavRight_en', '', '            <a class="twitter-timeline"  href="https://twitter.com/ETFLibrary" data-widget-id="702160486524186624">Tweets by @ETFLibrary</a>
            <script>!function(d,s,id){var js,fjs=d.getElementsByTagName(s)[0],p=/^http:/.test(d.location)?'http':'https';if(!d.getElementById(id)){js=d.createElement(s);js.id=id;js.src=p+"://platform.twitter.com/widgets.js";fjs.parentNode.insertBefore(js,fjs);}}(document,"script","twitter-wjs");</script>
          ')"] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 18220.

In this example, OpacNavRight contains a script to embed a Twitter timeline.
Comment 1 Jonathan Druart 2019-06-11 14:28:52 UTC
Created attachment 90494 [details] [review]
Bug 23093: (bug 22318 follow-up) Use placeholders in query moving OpacNavRight

Placeholders must be used (!) in all SQL queries.

Test plan:
1/ % git checkout 0c95e22afe13349b60d134217fce6718bfa9727f # commit before bug 22318
2/ Edit the pref OpacNavRight with something with "
  just a " and more '
3/ git checkout master
4/ restart_all
5/ run updatedatabase.pl
=> Boom and the content of the pref has been lost
6/ Repeat 1 to 3
7/ Apply this patch
8/ run updatedatabase.pl
=> The pref has been correctly moved to opac_news
Comment 2 Mark Tompsett 2019-06-11 14:59:21 UTC
Created attachment 90500 [details] [review]
Bug 23093: (bug 22318 follow-up) Use placeholders in query moving OpacNavRight

Placeholders must be used (!) in all SQL queries.

Test plan:
1/ % git checkout 0c95e22afe13349b60d134217fce6718bfa9727f # commit before bug 22318
2/ Edit the pref OpacNavRight with something with "
  just a " and more '
3/ git checkout master
4/ restart_all
5/ run updatedatabase.pl
=> Boom and the content of the pref has been lost
6/ Repeat 1 to 3
7/ Apply this patch
8/ run updatedatabase.pl
=> The pref has been correctly moved to opac_news

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 3 Nick Clemens 2019-06-12 13:55:46 UTC
Created attachment 90531 [details] [review]
Bug 23093: (bug 22318 follow-up) Use placeholders in query moving OpacNavRight

Placeholders must be used (!) in all SQL queries.

Test plan:
1/ % git checkout 0c95e22afe13349b60d134217fce6718bfa9727f # commit before bug 22318
2/ Edit the pref OpacNavRight with something with "
  just a " and more '
3/ git checkout master
4/ restart_all
5/ run updatedatabase.pl
=> Boom and the content of the pref has been lost
6/ Repeat 1 to 3
7/ Apply this patch
8/ run updatedatabase.pl
=> The pref has been correctly moved to opac_news

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 4 Martin Renvoize 2019-06-13 11:07:45 UTC
Nice work!

Pushed to master for 19.11.00
Comment 5 Fridolin Somers 2019-06-17 07:50:45 UTC
Pushed to 19.05.x for 19.05.01