OWASP / OWASP/SecurityShepherd
Refactor remaining Getter.java JDBC sites to try-with-resources
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 1.5k
- Forks
- 515
- Avg merge
- 3h 46m
- Merged PRs (30d)
- 1
Description
Summary
Refactor the remaining JDBC resource sites in src/main/java/dbProcs/Getter.java to use try-with-resources, so every PreparedStatement / CallableStatement / ResultSet is deterministically closed.
This is the Getter.java counterpart to the work already completed for the SSO path (#840 / PR #845) and for Setter.java (#846 / PR #852 — Setter.java is now clean). It is the last large file still triggering the recurring "Potential database resource leak" code-scanning comments seen on #816.
Why this is one issue, not a trickle
The code-quality bot reviews each push's diff, so partial fixes keep surfacing "new" comments on untouched regions of Getter.java while stale comments linger on code that has since moved. The fix is a single comprehensive pass over the file so the scan goes quiet in one shot. The scope is finite and enumerated below — there is a definite end.
Scope — 44 methods with genuine leak sites
Each creates a statement/ResultSet in the method body (outside a try (...) resource header). Clean pattern to apply (already used by e.g. getClassInfo, authUser, findPlayerById):
try (Connection conn = Database.getCoreConnection(ApplicationRoot);
CallableStatement callstmt = conn.prepareCall("...")) {
try (ResultSet rs = callstmt.executeQuery()) {
...
}
}
SIMPLE — single statement + ResultSet, straightforward wrap (42)
getClassCount, getClassInfo(String,String), getCsrfForumWithIframe, getCsrfForumWithImg, getFeedback, getIncrementalModules, getIncrementalModulesWithoutScript, getLessons, getModuleAddress, getModuleCategory, getModuleHash, getModuleIdFromHash, getModuleKeyType, getModuleNameLocaleKey, getModuleResult, getModuleResultFromHash, getModulesInOptionTags, getModulesInOptionTagsCTF, getModuleSolution, getModuleStatusMenu, getOpenCloseCategoryMenu, getProgress, getProgressJSON, getTournamentModules, getUserClassFromName, getUserIdFromName, getUserName, isCsrfLevelComplete, findAdminById, getPlayerCheatStatus, getModuleLayout, getFeedbackStatus, getRegistrationStatus, getScoreboardStatus, getScoreboardClass, getStartTimeStatus, getStartTime, getLockTimeStatus, getLockTime, getEndTimeStatus, getEndTime, getDefaultClass
CONDITIONAL — statement assigned in if/else branches, needs care (1)
getJsonScore—CallableStatementinitialized tonull, then assigned in one of two branches (totalScoreboard()vsclassScoreboard(?));ResultSetalso created in body. Cannot go straight into a TWR header — wrap after the branch, or hoist the SQL selection.
PARTIAL MITIGATION — ResultSet manually closed, statement still leaks (1)
isModuleOpen— has a manualrs.close()but thePreparedStatement/CallableStatementis not closed.
Acceptance criteria
- All 44 methods wrap JDBC resources in try-with-resources; SQL and branching unchanged (no behavior change).
- No remaining body-level statement/ResultSet creation in
Getter.java(CSRF/challenge behavior preserved). - "Potential database resource leak" code-scanning comments for
Getter.javacease. -
GetterITand challenge integration tests pass (run viaworkflow_dispatch"Build and Test" if on a feature→feature branch).
Notes
Setter.javais already fully converted (#846 / PR #852) — this issue isGetter.javaonly.getJsonScoreandisModuleOpenwarrant targeted review for behavior preservation.
Inventory generated by static analysis of the current dev#536 tree; line numbers omitted as they drift, method names are stable.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/main/java/dbProcs/Getter.java and review the 44 methods listed in the issue, beginning with the straightforward sites before examining getJsonScore and isModuleOpen. Preserve SQL, branching, and CSRF/challenge behavior while ensuring every statement and ResultSet uses try-with-resources. Run GetterIT and the challenge integration tests, using the Build and Test workflow if needed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, database, testing
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100