On bug 22715 we added a filter for circulation notes - this should probably be applied to all the fields on the page for consistency/safety
*** Bug 22484 has been marked as a duplicate of this bug. ***
Created attachment 89267 [details] [review] Bug 22781: Add tests Test plan: Apply only this patch => The tests will fail, the library's name is not correctly escaped. Note: This applies for the whole patchset, when the tests fail you will need to remove the patrons added by the tests with the following SQL query: delete from borrowers where surname like "test_patron_%"; We are expecting END to be called even if something goes wrong, but DESTROY must be used instead. This will be fixed separately, on its own bug report.
Created attachment 89268 [details] [review] Bug 22781: Escape cardnumber, category's description, library's name and dateexpiry This will fix the previous failure. Note that other fields like borrowernumber, Price escaped values, integers, etc. could be escaped the same way but will be useless (save polar bears).
Created attachment 89269 [details] [review] Bug 22781: Test patron's info filtering The patron's data was not correctly escaped actually. Test plan: This is a good value for the whole patchset: <strong>fir's"tname</strong> \123 ❤ use it for a patron's firstname, then do a search
Created attachment 89270 [details] [review] Bug 22781: Does not double filter address Ho, a bug! Some fields in the address where double filtered. Test plan: Use the evil strings in the address field and do a search, you will see double html encoded characters without this patch.
Created attachment 89271 [details] [review] Bug 22781: Correctly filter the emails Ho, another bug! The email was not correctly filtered. Test plan: Use the evil string for an email, do a search. => Notice that without this patch the results never show up
Just to point out, the interface won't actually allow saving the "bad string email," so you've got to force it in there in the DB. It's probably still good to make sure this is escaped though, because who knows what can happen in migration.
Created attachment 89395 [details] [review] Bug 22781: Add tests Test plan: Apply only this patch => The tests will fail, the library's name is not correctly escaped. Note: This applies for the whole patchset, when the tests fail you will need to remove the patrons added by the tests with the following SQL query: delete from borrowers where surname like "test_patron_%"; We are expecting END to be called even if something goes wrong, but DESTROY must be used instead. This will be fixed separately, on its own bug report. NOTE: I (Liz) did not test this first patch because my selenium was not working, that is why my SO line is not on the patch. If someone could do that quickly I'd be really grateful.
Created attachment 89396 [details] [review] Bug 22781: Escape cardnumber, category's description, library's name and dateexpiry This will fix the previous failure. Note that other fields like borrowernumber, Price escaped values, integers, etc. could be escaped the same way but will be useless (save polar bears). Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 89397 [details] [review] Bug 22781: Test patron's info filtering The patron's data was not correctly escaped actually. Test plan: This is a good value for the whole patchset: <strong>fir's"tname</strong> \123 ❤ use it for a patron's firstname, then do a search Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 89398 [details] [review] Bug 22781: Does not double filter address Ho, a bug! Some fields in the address where double filtered. Test plan: Use the evil strings in the address field and do a search, you will see double html encoded characters without this patch. Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 89399 [details] [review] Bug 22781: Correctly filter the emails Ho, another bug! The email was not correctly filtered. Test plan: Use the evil string for an email, do a search. => Notice that without this patch the results never show up Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 89582 [details] [review] Bug 22781: Add tests Test plan: Apply only this patch => The tests will fail, the library's name is not correctly escaped. Note: This applies for the whole patchset, when the tests fail you will need to remove the patrons added by the tests with the following SQL query: delete from borrowers where surname like "test_patron_%"; We are expecting END to be called even if something goes wrong, but DESTROY must be used instead. This will be fixed separately, on its own bug report. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 89583 [details] [review] Bug 22781: Escape cardnumber, category's description, library's name and dateexpiry This will fix the previous failure. Note that other fields like borrowernumber, Price escaped values, integers, etc. could be escaped the same way but will be useless (save polar bears). Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 89584 [details] [review] Bug 22781: Test patron's info filtering The patron's data was not correctly escaped actually. Test plan: This is a good value for the whole patchset: <strong>fir's"tname</strong> \123 ❤ use it for a patron's firstname, then do a search Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 89585 [details] [review] Bug 22781: Does not double filter address Ho, a bug! Some fields in the address where double filtered. Test plan: Use the evil strings in the address field and do a search, you will see double html encoded characters without this patch. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 89586 [details] [review] Bug 22781: Correctly filter the emails Ho, another bug! The email was not correctly filtered. Test plan: Use the evil string for an email, do a search. => Notice that without this patch the results never show up Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
NOTE FOR RM: I still have trouble running Selenium tests (server won't start right now). So I couldn't test them :(
Selenium Tests all pass :), Katrin gave me a heads up on IRC to run them for her.
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.06
Pushed to 18.05 for 18.04.14
*** Bug 18765 has been marked as a duplicate of this bug. ***