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

@DateTimeFormat's JSR-310 formatter is not strict in case of pattern [SPR-13567] #18143

Closed
spring-projects-issues opened this issue Oct 13, 2015 · 5 comments

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Oct 13, 2015

Toshiaki Maki opened SPR-13567 and commented

@DateTimeFormat's formatter is not strict in case of pattern attribute specified with JSR-310 Date&Time API on JDK 8.

For example, the following Form#date will be valid even though the requested value is 2015-02-29. It will be regarded as 2015-02-28!

public class Form {
  @DateTimeFormat(pattern = "yyyy-MM-dd")
  private java.time.LocalDate date;
  // ...
}

In case of java.util.Date or org.joda.time.LocalDate, 2015-02-29 is invalid because it's checked by strict mode.

This is because DateTimeFormatterFactory produces java.time.format.DateTimeFormatter using DateTimeFormatter#ofPattern.

https://github.com/spring-projects/spring-framework/blob/master/spring-context/src/main/java/org/springframework/format/datetime/standard/DateTimeFormatterFactory.java#L176-L178

In this case, java.time.format.ResolverStyle is not STRICT but SMART.

On the other hand, when iso attribute is specified, DateTimeFormatter.ISO_XXX is used and this java.time.format.ResolverStyle is STRICT.
So @DateTimeFormat(iso = DateTimeFormatter.ISO_DATE) works fine.

I think setting ResolverStyle.STRICT is good to keep consistency of this API like following:

dateTimeFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd")
        .withResolverStyle(ResolverStyle.STRICT);

This will be caused by the difference between Date&Time API and Joda-Time.
The following sample represents this issue plainly.

java.time.format.DateTimeFormatter dateTimeFormatter = DateTimeFormatter
        .ofPattern("yyyy-MM-dd");
System.out.println(dateTimeFormatter.parse("2015-02-29")); // valid

org.joda.time.format.DateTimeFormatter formatter = DateTimeFormat
        .forPattern("yyyy-MM-dd");
System.out.println(formatter.parseDateTime("2015-02-29")); // invalid

Affects: 4.1.7, 4.2.1

Issue Links:

  • #18144 @DateTimeFormat cannot accept Japanese Calendar (Wareki)
  • #18775 @RequestHeader doesn't accept RFC-1123 for conversion to java.time.Instant

2 votes, 5 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 13, 2015

Juergen Hoeller commented

Unfortunately, ResolverMode.STRICT is too strict for our purposes here: It even rejects some input values for conversion to LocalDateTime that are perfectly valid, apparently due to a strict rule for time zone handling against a pattern...

We'll try to sort this out for 4.3, based on custom processing logic if necessary, but I'm afraid this cannot be a candidate for 4.2.2 and 4.1.8 with such a risk for regressions.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 13, 2015

Toshiaki Maki commented

Hmm, it sounds more difficult than I thought.
iso attribute has been already strict.
Is it OK?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 13, 2015

Juergen Hoeller commented

It does not seem to cause any harm for the ISO mode, rather just for pattern mode.

The concrete test that fails on our side is DateTimeFormattingTests.testBindDateTimeAnnotatedPattern. Adding withResolverStyle(ResolverStyle.STRICT) in DateTimeFormatterFactory makes it fail with a pretty strange exception when it tries to extract a LocalDateTime from the parsed representation.

Unfortunately, I have not enough time to dig into this on my end, since we have more urgent fixes to consider for 4.2.2 and 4.1.8 still. If you can find out a bit more about how strict processing behaves there and how we could possibly fine tune it, please let me know!

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 13, 2015

Toshiaki Maki commented

What y in JSR-310 stands for is different from that of classic Date API.

https://docs.oracle.com/javase/8/docs/api/java/time/format/DateTimeFormatter.html

y means not "year" but "year-of-era". This difference affects in case of STRICT mode.
So i think , as a pattern of year with JSR-310, using u is appropriate.

I've send PR to fix this issue.
#889

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 23, 2016

Juergen Hoeller commented

Since our pattern definition follows the original java.text.SimpleDateFormat style, as also supported by Joda-Time, we need to preserve the meaning of the 'y' character as a general year in a "yy" or "yyyy" specification. For consistency with standard date parsing and Joda-Time parsing, we also need to parse in java.time's strict mode. The easiest way to achieve those semantics is to replace such year specifications with 'u' characters before parsing, which we're doing as of 4.3 now. This shouldn't conflict with any other intentions of using 'y', since year-of-era specifications do not successfully parse into the common value types anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants