Summary: | Multiple uses of javascript eval on ajax responses in acq.js | ||
---|---|---|---|
Product: | Koha | Reporter: | Hugh Davenport <hugh> |
Component: | Acquisitions | Assignee: | Hugh Davenport <hugh> |
Status: | RESOLVED DUPLICATE | QA Contact: | |
Severity: | major | ||
Priority: | P5 - low | CC: | chris, jonathan.druart, kyle, veron |
Version: | master | ||
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: | ||
Attachments: |
bug 9411 Fix javascript evals to use parseInt and parseFloat
bug 9411 Fix javascript evals to use parseInt and parseFloat bug 4439 Change some helper files to use C4::Service |
Description
Hugh Davenport
2013-01-17 03:26:45 UTC
A brief look tells me that it is checking for numerical types, which would be better suited by parseInt and parseFloat fixing Created attachment 14648 [details] [review] bug 9411 Fix javascript evals to use parseInt and parseFloat The acq.js file used evals to parse the return text of ajax queries to determine what result to return. This is a bad thing, and can potentially cause security risks. This patch converts those eval calls to parseInt and parseFloat calls. Note: This patch depends on the patch in bug 4437 so there are no conflicts. Signed-off-by: Hugh Davenport <hugh@davenport.net.nz> Created attachment 14650 [details] [review] bug 9411 Fix javascript evals to use parseInt and parseFloat The acq.js file used evals to parse the return text of ajax queries to determine what result to return. This is a bad thing, and can potentially cause security risks. This patch converts those eval calls to parseInt and parseFloat calls. Note: This patch depends on the patch in bug 4437 so there are no conflicts. Signed-off-by: Hugh Davenport <hugh@davenport.net.nz> Created attachment 14652 [details] [review] bug 4439 Change some helper files to use C4::Service Depends on bug 4437 to avoid conflicts Depends on bug 9412 for a new feature used Also fixes bug 9411 Signed-off-by: Hugh Davenport <hugh@davenport.net.nz> Hugh, just a note, the patches you are submitting appear to each have a sign-off line from yourself. Yep, that's quite common in a lot of projects, you'll notice Galen does it also. It means they have tested it themselves. Still needs an external sign off of course. In fact just the other day Galen remarked something like "when did we stop adding a sign off to our patches?" Hugh, can you add a test plan to the patches comments? Failing QA pending the addition of a test plan. Hey Kyle/Owen, What would the format of a test plan be? Cheers, Hugh Since the patch in question doesn't change the behavior of Koha ( at least in theory ) it would be as simple as 1) Perform some action that tests the code to be modified 2) Apply this patch 3) Perform the same action, ensure everything still works the same. What that the action or actions are, is up to you. Hugh, Any chances to see this resubmitted? Is this bug still valid? (Does not apply). *** This bug has been marked as a duplicate of bug 4439 *** |