Summary: | External track clicks links should get uri filtered | ||
---|---|---|---|
Product: | Koha | Reporter: | Aleisha Amohia <aleisha> |
Component: | OPAC | Assignee: | Aleisha Amohia <aleisha> |
Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | andrewfh, bolshawh, dcook, fridolin.somers, jonathan.druart |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
21.05.00,20.11.05
|
|
Bug Depends on: | 24699, 27979 | ||
Bug Blocks: | |||
Attachments: |
Bug 27961: Ensure external URLs are filtered
Bug 27961: Ensure external URLs are filtered Bug 27961: Ensure external URLs are filtered Bug 27961: [20.05.x] Ensure external URLs are filtered |
Description
Aleisha Amohia
2021-03-15 21:52:28 UTC
Created attachment 118256 [details] [review] Bug 27961: Ensure external URLs are filtered TT variables are used to build the link in items.uri . This is a problem particularly when TrackClicks is enabled because the items.uri value doesn't get escaped, resulting in a 404. To test: 1) Enable the TrackClicks system preference (set to Track or Track anonymously) 2) Find a record with an item 3) Edit the item and put an external URL under the Uniform Resource Identifier field 4) Go to that record in the OPAC 5) Click on Link to resource 6) Confirm you are not redirected to the URL and instead see a 404 7) Apply the patch and refresh the page 8) Click on Link to resource 9) Confirm you are redirected as expected 10) Go back to the staff client to edit your item. This time put two URLs in the Uniform Resource Identifier field, separated by ' | '. (Don't forget the spaces around the pipe, and don't forget to add a trailing slash to the URL). 11) Go back to the OPAC and refresh the page. 12) Confirm both URLs redirect as expected. Sponsored-by: Parliamentary Library New Zealand I feel like I've had an issue related to this but don't have the time to look at it right now... Hi Aleisha, I'm still getting some 404 errors when I add two URLs: e.g. https://bbc.co.uk/ | https://gu.com/ https://bbc.co.uk | https://gu.com https://bbc.co.uk | https://gu.com/ Sometimes one link will work but not the other one. Can you provide some examples of links that you think should work? Also for the test plan, do we need to test "trackclicks:track" and "trackclicks:track anonymously" separately or doesn't it make a difference? (In reply to Henry Bolshaw from comment #3) > Hi Aleisha, I'm still getting some 404 errors when I add two URLs: > > e.g. > > https://bbc.co.uk/ | https://gu.com/ > > https://bbc.co.uk | https://gu.com > > https://bbc.co.uk | https://gu.com/ > > Sometimes one link will work but not the other one. > > Can you provide some examples of links that you think should work? Actually this is a good point, it looks like it is broken in master as well (without applying this patch). I'll see if I can fix it with this patch. > > Also for the test plan, do we need to test "trackclicks:track" and > "trackclicks:track anonymously" separately or doesn't it make a difference? From my understanding, it doesn't make a difference. I've written a patch for Bug 27979 to address the broken multiple URLs bug. This patch can be tested on top of that one Created attachment 119091 [details] [review] Bug 27961: Ensure external URLs are filtered TT variables are used to build the link in items.uri . This is a problem particularly when TrackClicks is enabled because the items.uri value doesn't get escaped, resulting in a 404. To test: 1) Enable the TrackClicks system preference (set to Track or Track anonymously) 2) Find a record with an item 3) Edit the item and put an external URL under the Uniform Resource Identifier field 4) Go to that record in the OPAC 5) Click on Link to resource 6) Confirm you are not redirected to the URL and instead see a 404 7) Apply the patch and refresh the page 8) Click on Link to resource 9) Confirm you are redirected as expected 10) Go back to the staff client to edit your item. This time put two URLs in the Uniform Resource Identifier field, separated by ' | '. (Don't forget the spaces around the pipe, and don't forget to add a trailing slash to the URL). 11) Go back to the OPAC and refresh the page. 12) Confirm both URLs redirect as expected. Sponsored-by: Parliamentary Library New Zealand Signed-off-by: David Nind <david@davidnind.com> Caused by commit 26d4b7fd5e35f784f951c4f9e9bb084a9f4612a4 Bug 24699: Split items.uri in OPAC detail view ? Marcel, can you have a look at it please? (In reply to Jonathan Druart from comment #7) > Caused by > commit 26d4b7fd5e35f784f951c4f9e9bb084a9f4612a4 > Bug 24699: Split items.uri in OPAC detail view > ? > > Marcel, can you have a look at it please? Will do QAing Created attachment 119694 [details] [review] Bug 27961: Ensure external URLs are filtered TT variables are used to build the link in items.uri . This is a problem particularly when TrackClicks is enabled because the items.uri value doesn't get escaped, resulting in a 404. To test: 1) Enable the TrackClicks system preference (set to Track or Track anonymously) 2) Find a record with an item 3) Edit the item and put an external URL under the Uniform Resource Identifier field 4) Go to that record in the OPAC 5) Click on Link to resource 6) Confirm you are not redirected to the URL and instead see a 404 7) Apply the patch and refresh the page 8) Click on Link to resource 9) Confirm you are redirected as expected 10) Go back to the staff client to edit your item. This time put two URLs in the Uniform Resource Identifier field, separated by ' | '. (Don't forget the spaces around the pipe, and don't forget to add a trailing slash to the URL). 11) Go back to the OPAC and refresh the page. 12) Confirm both URLs redirect as expected. Sponsored-by: Parliamentary Library New Zealand Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 21.05, thanks to everybody involved! Pushed to 20.11.x for 20.11.05 I'm not recreating the issue when following your test plan on 20.05. Additionally, the patch doesn't apply cleanly. Please rebase if needed. Created attachment 120426 [details] [review] Bug 27961: [20.05.x] Ensure external URLs are filtered TT variables are used to build the link in items.uri . This is a problem particularly when TrackClicks is enabled because the items.uri value doesn't get escaped, resulting in a 404. To test: 1) Enable the TrackClicks system preference (set to Track or Track anonymously) 2) Find a record with an item 3) Edit the item and put an external URL under the Uniform Resource Identifier field 4) Go to that record in the OPAC 5) Click on Link to resource 6) Confirm you are not redirected to the URL and instead see a 404 7) Apply the patch and refresh the page 8) Click on Link to resource 9) Confirm you are redirected as expected 10) Go back to the staff client to edit your item. This time put two URLs in the Uniform Resource Identifier field, separated by ' | '. (Don't forget the spaces around the pipe, and don't forget to add a trailing slash to the URL). 11) Go back to the OPAC and refresh the page. 12) Confirm both URLs redirect as expected. Sponsored-by: Parliamentary Library New Zealand |