Skip to content

fix(rest): validate department log date as yyyy-MM-dd before Thrift call#4041

Open
Shivamrut wants to merge 1 commit intoeclipse-sw360:mainfrom
Shivamrut:fix/bug/Sw360DepartmentService
Open

fix(rest): validate department log date as yyyy-MM-dd before Thrift call#4041
Shivamrut wants to merge 1 commit intoeclipse-sw360:mainfrom
Shivamrut:fix/bug/Sw360DepartmentService

Conversation

@Shivamrut
Copy link
Copy Markdown
Contributor

Summary

This PR fixes date validation for GET department log file content by date (GET …/departments/logFileContent?date=…), implemented in Sw360DepartmentService.getLogFileContentByDate.

Background

The REST layer is supposed to ensure the date query parameter is a strict calendar date in yyyy-MM-dd (e.g. 2025-04-04) before calling UserService.getLogFileContentByName, so malformed or unexpected strings are not passed through to the user/thrift layer. On failure, callers should receive a clear SW360Exception with: Invalid date time format, must be: yyyy-MM-dd.

What was wrong (root cause)

1) Wrong CommonUtils predicate in isValidDate

  • The code used CommonUtils.isNotNullEmptyOrWhitespace(dateStr) in the first branch and returned false immediately when it was true.
  • isNotNullEmptyOrWhitespace is true when the string has non-blank content (i.e. it is not null/empty/whitespace-only).
  • So for every normal input (including valid dates like 2025-04-04 and garbage like bad-date), that branch ran and the method returned false without ever calling LocalDate.parse.
  • The LocalDate.parse(..., yyyy-MM-dd) path was therefore unreachable for typical non-empty parameters, so yyyy-MM-dd was never enforced in practice.

2) Caller condition did not match a sensible “valid date” meaning

  • getLogFileContentByDate used if (isValidDate(date)) { throw … invalid format } while the private helper was named isValidDate.
  • After fixing the predicate and parse logic, isValidDate means “parses as yyyy-MM-dd (returns true only then). The caller must reject when validation fails, i.e. if (!isValidDate(date)) { throw … }.

Together, these bugs meant invalid strings could still be forwarded to getLogFileContentByName, while the intended early validation path did not work reliably.

What this PR changes

isValidDate(String dateStr)

  • Return false if the value is null, empty, or only whitespace (CommonUtils.isNullEmptyOrWhitespace).
  • Otherwise parse with DateTimeFormatter pattern yyyy-MM-dd and LocalDate.parse.
  • Return true only if parsing succeeds; DateTimeParseExceptionfalse.

getLogFileContentByDate(String date)

  • if (!isValidDate(date)) → throw SW360Exception with Invalid date time format, must be: yyyy-MM-dd.
  • Only then call userClient.getLogFileContentByName(date) (unchanged thrift contract for valid input).

No new dependencies.

Regression and compatibility

Area Impact
Upstream DepartmentController passes the date request parameter straight into the service. No controller change required. Clients that already send yyyy-MM-dd (e.g. ?date=2025-04-04) see the same successful path: same string reaches getLogFileContentByName as before.
Downstream UserHandler.getLogFileContentByName still receives a yyyy-MM-dd-compatible name when the request succeeds. Behavior change is limited to invalid inputs: they are now rejected in the service with the documented exception instead of being passed through.
Strictness Only strings that LocalDate.parse accepts for yyyy-MM-dd succeed (e.g. no extra suffix, no alternate separators). This matches the documented format.

How to test (reviewer)

  1. Happy path: GET …/api/departments/logFileContent?date=2025-04-04 (with auth as required) → 200 and log lines when configured (same as before for valid dates).
  2. Invalid: ?date=bad-date or ?date=2025-13-40SW360Exception path (HTTP status depends on global exception handling; the service error message is fixed).
  3. Empty: missing or blank date (if reachable) → validation failure.

Suggest Reviewer

@GMishx @rudra-superrr @bibhuti230185 @KoukiHama

Signed-off-by: Shivamrut <gshivamrut@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant