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

DurationFormatterUtils should not attempt to parse an empty duration #33669

Conversation

Seungpang
Copy link
Contributor

Hi Team,

While testing the DurationFormatterUtils functionality, I came across a couple of issues that I'd like to address in this PR

  1. Empty String Handling in parse Method:

    • Currently, when an empty string ("", " ") is passed to the parseComposite(value) method, the result is PT0S instead of an exception being thrown. This doesn't seem like the correct behavior, as an empty string is an invalid input and returning PT0S can be misleading in this case.
  2. NullPointerException (NPE) in print Method:

    • The print method does not handle null values properly for the Duration parameter, resulting in a NullPointerException.

Changes

  • parse method:
    • Added validation for empty strings and null values.
  • print method:
    • Added validation for null Duration values.
  • Changed error handling to throw Exception instead of Throwable

Thank you for taking the time to review this PR! Please feel free to share any feedback or suggestions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 8, 2024
@snicoll snicoll added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 9, 2024
@snicoll snicoll added this to the 6.2.0-RC2 milestone Oct 9, 2024
@snicoll snicoll changed the title Fix null and empty string handling in DurationFormatterUtils DurationFormatterUtils should not attempt to parse an empty duration Oct 9, 2024
@snicoll snicoll self-assigned this Oct 9, 2024
snicoll pushed a commit that referenced this pull request Oct 9, 2024
@snicoll snicoll closed this in f54faba Oct 9, 2024
@snicoll
Copy link
Member

snicoll commented Oct 9, 2024

@Seungpang thanks for the PR. I've reverted the change on the print method as the current behavior is expected, considering that Duration is flagged as mandatory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants