Conversation
aronsky
left a comment
There was a problem hiding this comment.
Looks good overall, but there's a lot of repeated code. I would create an intermediate class that extends HttpServlet and provides implementations for doGet, goodSanitationForTag, and goodSanitationForIdAttribute (while we're at it, the last 2 should probably be called sanitizeTag and sanitizeAttribute).
Then, all the new BenchmarkTest classes can extend the above intermediate class, and only override the doPost method.
|
Hi Aronsky, basically in Benchmark structure for each test case there is a java file, an html file and an xml. If we need to create a intermediate class, we should optimize all these files. And also, the sanitization code is different for each scenario. Please suggest |
|
Why would creating an intermediate class require any changes to the HTML/XML files? And I see that there are 2 kinds of |
Hi Aronsky, Java does not support multiple inheritance. So the Intermediate class cannot extends HttpServelt. Please suggest! |
|
No multiple inheritance required. Create an intermediate class that extends |
aronsky
left a comment
There was a problem hiding this comment.
Great. However, Intermediate is not a good class name. Rename it to something like SanitizingHttpServlet.
And please remove the code that you commented out, which is no longer used.
aronsky
left a comment
There was a problem hiding this comment.
Great, looks good to me.
* XSS Scenarios * xss mods * review comments incorporated * scripts to create several SAST reports * Update AppScanDynamicReader2.java per pull request OWASP-Benchmark#157, with additional changes by me, including running spotless properly. * Update codeql-analysis.yml CodeQL auto build is failing with a "Picked up JAVA_TOOL_OPTIONS:..." message, so let's try a plain old Maven build. * Update codeql-analysis.yml Ensure that the CodeQL workflow checks out enough git history to support the spotless ratchetFrom test in the Maven build. * Update README.md Trivial grammar fix. * Renamed Intermediate class & removed the commented out code * change horusec to docker * prevent installation outputs * insider - check for availability and updates * use docker instead of calling semgrep directly * require docker * fix typo * Upgrade a bunch of dependencies and remove a few unused ones. * fix insider installation check 🙈 * execute bit for docker script * more preconditions checks * store sonarqube credentials in seperate file * prevent adding of sonarqube credentials * docker cleanup * Revert "docker cleanup" This reverts commit 70e7c48. * docker cleanup * Add dependency used by a script. Minor tweaks to a few scripts. * A few minor fixes to the other IAST run scripts. * This is a MAJOR change. This commit rips out all the utilities included with Benchmark to score it, run crawlers on it, etc. Those utilities are now in a seperate project called BenchmarkUtils. BenchmarkUtils produces a maven plugin that is now used by all the scoring and crawling scripts updated in this commit. You have to clone BenchmarkUtils, then run: mvn install, to get the plugin (which is built and installed locally). * Proxy Settings added to OWASP Benchmark Crawler to enable different hosts (#1) * Simplify configuration of 2 config files. Minor updates to a number of test cases. * Updated the Dockerfile to reflect the repo change to BenchmarkJava * Fix utility method so more 'standard' headers are filtered out when trying to identify the custom header for test cases that use that. * Intermediate file is deleted Co-authored-by: user <[email protected]> Co-authored-by: gituser <[email protected]> Co-authored-by: kaveti.l <[email protected]> Co-authored-by: Sascha Knoop <[email protected]> Co-authored-by: Dave Wichers <[email protected]> Co-authored-by: dandersonaspect <[email protected]> Co-authored-by: Arun Muthu <[email protected]> Co-authored-by: Arun Muthu <[email protected]> Co-authored-by: shivababuh <[email protected]>
No description provided.