[Bug 42047] New: Allow per-instance overrides for C4::Scrubber
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 Bug ID: 42047 Summary: Allow per-instance overrides for C4::Scrubber Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org Koha comes with a number of pre-defined C4::Scrubber profiles, but there are times where you might want to configure a different profile. Either for overriding built in profiles or for use with the Template::Toolkit scrubber filter. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=31934 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=40079, | |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=40087, | |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36328, | |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=40717, | |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=38498, | |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=40078, | |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=40077 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #1 from David Cook <dcook@prosentient.com.au> --- Created attachment 195038 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=195038&action=edit Bug 42047: Override configuration using html_scrubber.yaml This patch allows you to override any of the C4::Scrubber settings using a file called "html_scrubber.yaml" which is to be located in the same directory as your koha-conf.xml file. This allows people to override existing configurations in the event that they're not permissive enough (or are too permissive) e.g. The following YAML placed at /etc/koha/sites/kohadev/html_scrubber.yaml would make the additional_content scrubber only allow "div" --- additional_content: allow: ["div"] -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #2 from David Nind <david@davidnind.com> --- Not sure how to make this work. Step 8 is what I expected, but the other tags (p>, <b>, <em>) were allowed as well. Do you use disallow to not allow a tag? Here is what I did: 1. Added an HTML customisation for OpacLibraryInfo for all libraries: <p><strong>This is bold</strong></p> <p><em>This is italic</em></p> <div> <p>This is another paragraph with a div</p> </div> 2. Before the patch, look at the HTML/source for the Centerville library information page (OPAC > Libraries > Centerville) 3. Note that all the tags in step 1 are allowed and shown in the HTML. 4. Create the /etc/koha/sites/kohadev/html_scrubber.yaml file with: additional_content: allow: ["div"] 5. Apply the patch. 6. Restart everything: restart_all 7. Refresh the page. 8. Repeat step 2 - note that only the div is allowed, all the other markup us removed. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |no-sandbox Assignee|koha-bugs@lists.koha-commun |dcook@prosentient.com.au |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- (In reply to David Nind from comment #2)
Not sure how to make this work.
Step 8 is what I expected, but the other tags (p>, <b>, <em>) were allowed as well.
I'm so sorry, David. I didn't provide a good commit message. Originally, this was bundled together with a different bug which isn't part of main yet. There is no additional_content scrubber yet. I was testing this bug as if the other was a dependency, but I've posted it here because it's technically not a dependency. Anyway, I'll provide a better test plan now. My apologies for wasting your time! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- Test plan: 0. Do not apply the patch yet 1. Log into Koha, click on "Tools", click on "Record display customizations" 2. Click "New entry" 3. Add a "Publication date" in the past, "Appear in position": 1, and a "Title" of "test" 4. In "Content:" add the following: <p><strong>This is bold</strong></p> <p><em>This is italic</em></p> <div> <p>This is another paragraph with a div</p> </div> 5. Search the catalogue for "test" (or whatever you like) 6. Click on the title of the first result 7. Note under "OPAC view" there is all the HTML content that we added 8. Apply the patch 9. Create the /etc/koha/sites/kohadev/html_scrubber.yaml file with: --- record_display: allow: ["div"] 10. restart_all 11. Refresh the page 12. Note that that all the text appears but it is no longer styled 13. If you can view the source of the page, you'll notice that all the HTML we added was stripped out *except* the "div" element -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=42315 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #195038|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 196986 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196986&action=edit Bug 42047: Override configuration using html_scrubber.yaml This patch allows you to override any of the C4::Scrubber settings using a file called "html_scrubber.yaml" which is to be located in the same directory as your koha-conf.xml file. This allows people to override existing configurations in the event that they're not permissive enough (or are too permissive) e.g. The following YAML placed at /etc/koha/sites/kohadev/html_scrubber.yaml would make the additional_content scrubber only allow "div" Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196986|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 196987 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=196987&action=edit Bug 42047: Override configuration using html_scrubber.yaml This patch allows you to override any of the C4::Scrubber settings using a file called "html_scrubber.yaml" which is to be located in the same directory as your koha-conf.xml file. This allows people to override existing configurations in the event that they're not permissive enough (or are too permissive) Test plan: 0. Do not apply the patch yet 1. Log into Koha, click on "Tools", click on "Record display customizations" 2. Click "New entry" 3. Add: - Display location: OPACDetailPage - Publication date: in the past - Appear in position: 1 - Defult > Title: test 4. In the Content field add: <p><strong>This is bold</strong></p> <p><em>This is italic</em></p> <div> <p>This is another paragraph with a div</p> </div> 5. Search the catalogue in the OAPC for "test" (or whatever you like) 6. Click on the title of the first result 7. Note under "OPAC view" there is all the HTML content that we added 8. Apply the patch 9. Create the /etc/koha/sites/kohadev/html_scrubber.yaml file with: record_display: allow: ["div"] 10. restart_all 11. Refresh the page 12. Note that that all the text appears but it is no longer styled 13. If you can view the source of the page, you'll notice that all the HTML we added was stripped out *except* the "div" element Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This enhancement allows you release notes| |to override any of the HTML | |scrubber (C4::Scrubber) | |settings using a file | |called "html_scrubber.yaml" | |which is located in the | |same directory as your | |koha-conf.xml file. | | | |This allows libraries to | |override existing | |configurations in the event | |that they're not permissive | |enough (or are too | |permissive). | | | |Example for kohadev | |instance, only allow div | |tags: | | | |File to create: | |/etc/koha/sites/kohadev/htm | |l_scrubber.yaml | | | |Contents of the file: | | | |record_display: | | allow: ["div"] --- Comment #7 from David Nind <david@davidnind.com> --- Thanks David! I added the test plan to the patch. David -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- (In reply to David Nind from comment #7)
Thanks David!
I added the test plan to the patch.
David
Thanks, David. Much appreciated! I think that this will be a really useful change. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=40659 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=19838 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=41547 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=41086 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=39718 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=19852 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28387 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=42346 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Sponsorship status|--- |Unsponsored Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #196987|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201394 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201394&action=edit Bug 42047: Override configuration using html_scrubber.yaml This patch allows you to override any of the C4::Scrubber settings using a file called "html_scrubber.yaml" which is to be located in the same directory as your koha-conf.xml file. This allows people to override existing configurations in the event that they're not permissive enough (or are too permissive) Test plan: 0. Do not apply the patch yet 1. Log into Koha, click on "Tools", click on "Record display customizations" 2. Click "New entry" 3. Add: - Display location: OPACDetailPage - Publication date: in the past - Appear in position: 1 - Defult > Title: test 4. In the Content field add: <p><strong>This is bold</strong></p> <p><em>This is italic</em></p> <div> <p>This is another paragraph with a div</p> </div> 5. Search the catalogue in the OAPC for "test" (or whatever you like) 6. Click on the title of the first result 7. Note under "OPAC view" there is all the HTML content that we added 8. Apply the patch 9. Create the /etc/koha/sites/kohadev/html_scrubber.yaml file with: record_display: allow: ["div"] 10. restart_all 11. Refresh the page 12. Note that that all the text appears but it is no longer styled 13. If you can view the source of the page, you'll notice that all the HTML we added was stripped out *except* the "div" element Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201395 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201395&action=edit Bug 42047: (QA follow-up) Guard against invalid html_scrubber.yaml override_default_settings runs when C4::Scrubber is first loaded, so an unhandled YAML parse error (e.g. a typo in a hand-edited html_scrubber.yaml) would previously abort compilation of the module and take down every page that uses it. Wrap the load in try/catch so a broken override file falls back to the built-in scrubber defaults instead, and carp a warning identifying the file so the problem is visible in the logs. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201396 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201396&action=edit Bug 42047: (QA follow-up) Document override_default_settings and drop dead check Document the html_scrubber.yaml override mechanism in POD, including the full-replace-not-merge semantics per type and the restart requirement, so this isn't only discoverable via the bug's test plan. Also drop the redundant '$filename &&' truthiness check - $filename is always a non-empty string from sprintf. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 201397 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201397&action=edit Bug 42047: (QA follow-up) Add tests for override_default_settings Cover the cases that weren't previously exercised: no override file present, a valid override replacing a scrubber type, a malformed override file falling back to defaults and warning (bug 42047 QA follow-up), and non-hash override content being ignored. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- Many thanks, Martin! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42047 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA CC| |pedro.amorim@openfifth.co.u | |k --- Comment #14 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Hi guys, the guard here is scoped to "a typo in a hand-edited html_scrubber.yaml" taking down every page, but it only catches unparseable YAML and a wrong top-level type, e.g.: 1) Create /etc/koha/sites/kohadev/html_scrubber.yaml with: record_display: - div - span 2) restart_all 3) In the staff interface: Tools → Record display customizations → New entry - Display location: OPACDetailPage - Publication date: in the past - Appear in position: 1 - Content: <p>test</p> - Save - Notice you get a Not a HASH reference error 4) Access a record on the OPAC: http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=76 Notice you get the same Not a HASH reference error -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org