OWASP / OWASP/SecurityShepherd

Refactor remaining Getter.java JDBC sites to try-with-resources

Open
#855 0 comments 0 reactions 0 assignees View on GitHub

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)
  • getJsonScoreCallableStatement initialized to null, then assigned in one of two branches (totalScoreboard() vs classScoreboard(?)); ResultSet also 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 manual rs.close() but the PreparedStatement/CallableStatement is 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.java cease.
  • GetterIT and challenge integration tests pass (run via workflow_dispatch "Build and Test" if on a feature→feature branch).

Notes

  • Setter.java is already fully converted (#846 / PR #852) — this issue is Getter.java only.
  • getJsonScore and isModuleOpen warrant 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.