If you search for "this & that" and delete one of the results, after it is deleted the search in the reloaded page is for "this " rather than for "this & that". The template uses the url filter, which is for filtering an entire URI where :, &, #, etc. should be left alone, rather than the uri filter which is for filtering a URI component where they should be encoded.
Created attachment 111184 [details] [review] Bug 26606: Correctly URI-encode query string in URL loaded after deleting an authority record If you search for "this & that" and delete one of the results, after it is deleted the search in the reloaded page is for "this " rather than for "this & that". The template uses the url filter, which is for filtering an entire URI where :, &, #, etc. should be left alone, rather than the uri filter which is for filtering a URI component where they should be encoded. To test: 1) Go to Authorities > New authority > New from Z39.50/SRU 2) Search for the Author (corporate) this & that collective 3) Actions > Import, Save 4) From the detail for that authority, Edit > Edit as new (duplicate) 5) Edit the 110 field, add 2 at the end and Save 6) Search authorities for this & that 7) For one of the ones you created, Actions > Delete, confirm 8) The page that reloads should have a search for this & that, not just for this
Created attachment 111208 [details] [review] Bug 26606: Correctly URI-encode query string in URL loaded after deleting an authority record If you search for "this & that" and delete one of the results, after it is deleted the search in the reloaded page is for "this " rather than for "this & that". The template uses the url filter, which is for filtering an entire URI where :, &, #, etc. should be left alone, rather than the uri filter which is for filtering a URI component where they should be encoded. To test: 1) Go to Authorities > New authority > New from Z39.50/SRU 2) Search for the Author (corporate) this & that collective 3) Actions > Import, Save 4) From the detail for that authority, Edit > Edit as new (duplicate) 5) Edit the 110 field, add 2 at the end and Save 6) Search authorities for this & that 7) For one of the ones you created, Actions > Delete, confirm 8) The page that reloads should have a search for this & that, not just for this Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
Looks like all the other filters shown in the context should be changed to "uri" from "html" as well, but seems out of scope for this change.
A bug (not this bug) about properly escaping authtypecode everywhere it is used really ought to exist, but as for e.g. orderby which has four possible values, HeadingDsc or HeadingAsc or null or an XSS attack in a spearphishing link, I'm unable to come up with any scenario where it would be valuable to URI-escape the quote that starts the XSS attack as %22 so it would be carefully passed through to the search that reloads after a deletion rather than HTML-escaping it as " and letting UA error handling deal with a bogus " URI param.
(In reply to Phil Ringnalda from comment #4) > A bug (not this bug) about properly escaping authtypecode everywhere it is > used really ought to exist In this case, I think a lot of those "html" filters were added by an automated script to add XSS protection. Not sure why the value would've had a "url" filter instead of a "uri" filter. Probably just human error. > I'm unable to come up with any scenario where it would be valuable to > URI-escape the quote that starts the XSS attack as %22 so it would be > carefully passed through to the search that reloads after a deletion rather > than HTML-escaping it as " and letting UA error handling deal with a > bogus " URI param. I do not understand what you're trying to say here. As for my earlier comment, I was saying that we should be using uri filters instead of html filters when URI building. But yes not in this bug.
Happy to discuss things which are not about this bug in a place which is not in this bug.
(In reply to Phil Ringnalda from comment #6) > Happy to discuss things which are not about this bug in a place which is not > in this bug. I think that I'm finished participating in this discussion, but feel free to open up more bug reports for URI encoding the rest of the query string arguments and authtypecode as you said.
Created attachment 111749 [details] [review] Bug 26606: Correctly URI-encode query string in URL loaded after deleting an authority record If you search for "this & that" and delete one of the results, after it is deleted the search in the reloaded page is for "this " rather than for "this & that". The template uses the url filter, which is for filtering an entire URI where :, &, #, etc. should be left alone, rather than the uri filter which is for filtering a URI component where they should be encoded. To test: 1) Go to Authorities > New authority > New from Z39.50/SRU 2) Search for the Author (corporate) this & that collective 3) Actions > Import, Save 4) From the detail for that authority, Edit > Edit as new (duplicate) 5) Edit the 110 field, add 2 at the end and Save 6) Search authorities for this & that 7) For one of the ones you created, Actions > Delete, confirm 8) The page that reloads should have a search for this & that, not just for this Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 20.11, thanks to everybody involved!
backported to 20.05.x for 20.05.06
backported to 19.11.x for 19.11.12
Backported to 19.05.x branch for 19.05.17