Skip to content

ascanrulesBeta: More example alerts #5626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Conversation

kingthorin
Copy link
Member

@kingthorin kingthorin commented Aug 4, 2024

Overview

  • CHANGELOG > Add notes.
  • Scan rules > Add example alert functionality (6119).
  • Unit tests > Assert the new example alerts.

Related Issues

Checklist

  • [na] Update help
  • Update changelog
  • Run ./gradlew spotlessApply for code formatting
  • Write tests
  • Check code coverage
  • Sign-off commits
  • Squash commits
  • Use a descriptive title

@thc202
Copy link
Member

thc202 commented Aug 5, 2024

The build is failing.

 SlackerCookieScanRuleUnitTest > shouldHaveExpectedExampleAlert() FAILED
    java.lang.AssertionError: 
    Expected: is "Cookies that don't have expected effects can reveal flaws in application logic. In the worst case, this can reveal where authentication via cookie token(s) is not actually enforced.\nThese cookies affected the response: oops\nThese cookies did NOT affect the response: bar,foo\n"
         but: was "Cookies that don't have expected effects can reveal flaws in application logic. In the worst case, this can reveal where authentication via cookie token(s) is not actually enforced.\nThese cookies affected the response: oops\nThese cookies did NOT affect the response: foo,bar\n"
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
        at org.zaproxy.zap.extension.ascanrulesBeta.SlackerCookieScanRuleUnitTest.shouldHaveExpectedExampleAlert(SlackerCookieScanRuleUnitTest.java:79)

@thc202
Copy link
Member

thc202 commented Aug 8, 2024

How was the test fixed?

There's a typo in the add-on ID.

@kingthorin kingthorin changed the title ascanruelsBeta: More example alerts ascanrulesBeta: More example alerts Aug 8, 2024
@kingthorin
Copy link
Member Author

kingthorin commented Aug 8, 2024

I fixed the expected string. I had assumed the cookie collection was processed in order but apparently it's processed backwards 🤷‍♂️

(Or maybe not backwards but ordered...)

@thc202
Copy link
Member

thc202 commented Aug 8, 2024

The push does not have any changes(?). The set used does not have a reliable ordering.

@kingthorin
Copy link
Member Author

Oh I didn't realize I'd just been lucky. I had done multiple runs locally but not any statistically significant number.

Shall I switch specifically to a SortedSet for the example? Or use string starts with for the test?

@thc202
Copy link
Member

thc202 commented Aug 8, 2024

Better with SortedSet, we do need a predictable/constant ordering otherwise we'll have unwanted changes in the website.

- CHANGELOG > Add notes.
- Scan rules > Add example alert functionality (6119).
- Unit tests > Assert the new example alerts.

Signed-off-by: kingthorin <[email protected]>
@kingthorin
Copy link
Member Author

Done

@thc202
Copy link
Member

thc202 commented Aug 11, 2024

Thank you!

@psiinon psiinon merged commit 5a93596 into zaproxy:main Aug 12, 2024
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 12, 2024
@kingthorin kingthorin deleted the ab-examples branch August 12, 2024 09:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants