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

#235 Replace usage of java.util.Date #884

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

pveeckhout
Copy link

Replace usage of java.util.Date with java.time.Instant in JWT claims

This commit replaces the usage of java.util.Date with java.time.Instant in handling JWT claims. This means upgrading from JDK7 to JDK8. This is done for better adherence to ISO 8601 standards and better precision in time-related operations. Classes working with claims have been updated to use the newer Instant class, providing better precision and cross-timezone compatibility. Additionally, overloading convenience methods for setting OffsetDateTime and ZonedDateTime on top of Instant have been added on the JwtBuilder and DefaultJwtBuilder.

Closes #235

…T claims

This commit replaces the usage of java.util.Date with java.time.Instant in handling JWT claims. This means upgrading from JDK7 to JDK8. This is done for better adherence to ISO 8601 standards and better precision in time-related operations. Classes working with claims have been updated to use the newer Instant class, providing better precision and cross-timezone compatibility. Additionally, overloading convenience methods for setting OffsetDateTime and ZonedDateTime on top of Instant have been added on the JwtBuilder and DefaultJwtBuilder.
@pveeckhout
Copy link
Author

pveeckhout commented Dec 25, 2023

There are 2 failing tests:

io.jsonwebtoken.JwtParserTest#testParseRequireCustomInstant_Success
io.jsonwebtoken.JwtParserTest#testParseRequireCustomInstant_Incorrect_Fail

If a custom claim is added with type instant, then there are issues in deserializing it. The Instant is saved as a numeric date according to the RFC standard (epochSeconds), while io.jsonwebtoken.impl.lang.JwtDateConverter#toInstant expected it to be in epochMillis.

A decision needs to be made how this needs to be handled.

Pieter Van Eeckhout and others added 4 commits December 26, 2023 10:14
The JWT handling has been updated to default to Java's Instant class. This change removed support for various date and time classes such as ZonedDateTime, OffsetDateTime, Date, and Calendar in favor of an Instant-based approach throughout the codebase. The appropriate tests and documentation were updated to reflect this change.
Replaced all instances of 'Date' with 'Instant' in JwtParserTest to more accurately reflect variable usage. Also added a TODO in JwtDateConverter.java to clarify the handling of epochMillis vs epochSeconds.
This commit updates the version of the project across all pom.xml files from 0.12.4-SNAPSHOT to 1.0.0-SNAPSHOT as it prepares for a major release. Changes cover the core project and various extensions, signaling a coordinated upgrade across the codebase.
All instances of java.util.Date in code have been replaced with java.time.Instant for better time precision and timezone handling. This includes changes in variable names, method names, test cases, comments and documentation. The java.util.Date-based utilities have also been removed.
@lhazlewood
Copy link
Contributor

@pveeckhout Thanks for taking an interest in helping with this!

However, we're not ready to force a JDK 8 upgrade until remaining JWT functionality is finished. #308 has some additional work in addition to the Java Date/Time changes for #235. #474 might also come into play instead of the existing builder utility methods as well. Some design work needs to be done before we can resolve this in earnest.

I'm happy to leave this PR open of course, but you might have to deal with changes/rebases as we make other changes first. I hope that's ok!

@pveeckhout
Copy link
Author

@lhazlewood, My pleasure to help! I am using jjwt in my own projects, and figured it might be time to try and contribute back to project.

I will gladly keep this PR up to date with the other incoming changes.

If there any other issue you figure I could help with, feel free to direct me to them.

@lhazlewood lhazlewood added the jdk8 Changes related to migrating to JDK8 API features label Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jdk8 Changes related to migrating to JDK8 API features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Java8 Date/Time formats
2 participants