Skip to content
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

Issue #387 Add deprecated jsp rule key #391

Merged
merged 2 commits into from
Nov 17, 2021

Conversation

gtoison
Copy link
Contributor

@gtoison gtoison commented Oct 20, 2021

Between version 3.3 and 3.4 of the plugin rules XSS_JSP_PRINT and XSS_REQUEST_PARAMETER_TO_JSP_WRITER have been moved from the Java findsecbugs repository to the JSP findsecbugs-jsp repository.

When a rule key is modified the plugin should apparently declare it's "deprecated key(s)" to let SonarQube know, see: https://javadocs.sonarsource.org/7.1/apidocs/org/sonar/api/server/rule/RulesDefinition.Rule.html#deprecatedRuleKeys--

The database was left in an apparently inconsistent state for SQ installations where the plugin had been upgraded from <= 3.3 to >= 3.4
Then when the API used to load profiles was changed in #369 (to make the plugin compatible with SQ 9) the server crashed on startup with error: BadRequestException: java rule findsecbugs:XSS_JSP_PRINT cannot be activated on jsp profile FindBugs Security JSP

This PR adds the "deprecated keys" (i.e. the old rules ID) for these two rules and prevent the plugin from trying to activate a removed rule into a profile.

@gtoison
Copy link
Contributor Author

gtoison commented Oct 31, 2021

Hello @KengoTODA,
Could you please review this PR? There are several duplicated issues for it, so lots of users are affected (#387 #389 and #393).
The checks fail because I do not have the right to run a SQ analysis.
Please let me know if something is unclear or needs to be adjusted.

Pull request #386 is also ready for review when someone has time

@gtoison gtoison requested a review from pethers November 15, 2021 08:43
@gtoison
Copy link
Contributor Author

gtoison commented Nov 15, 2021

@pethers could you please review this PR?
It solves the most pressing issue: the plugin causes the entire SQ server to crash at startup on installations that had version 3.3 or earlier and upgraded progressively to the current version

@pethers
Copy link
Contributor

pethers commented Nov 15, 2021

Not sure why https://github.com/spotbugs/sonar-findbugs/runs/3948483294?check_suite_focus=true is failing but will review the code changes.

Copy link
Contributor

@pethers pethers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me.

@gtoison
Copy link
Contributor Author

gtoison commented Nov 15, 2021

Thanks @pethers
If I understand correctly the action is running on my forked repo, and that repo does not have the permission to run the SonarQube analysis

@KengoTODA, is there a suggested workflow before merging PRs? Like getting two reviews, waiting a bit for potential comments, etc?

I looked at some old outstanding PR and if I hypothetically wanted to merge them I do not seem to have the rigths:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants