X Tutup
Skip to content

[SDK-3171] Fix header claims serialization#549

Merged
jimmyjames merged 2 commits intov4-devfrom
ser-header
Mar 16, 2022
Merged

[SDK-3171] Fix header claims serialization#549
jimmyjames merged 2 commits intov4-devfrom
ser-header

Conversation

@jimmyjames
Copy link
Copy Markdown
Contributor

Changes

Currently, we serialize the header claims using Jackson's default writeString. This causes date-time based claims to be serialized as milliseconds since the epoch, instead of the required seconds since the epoch.

This PR refactors much of the logic of the PayloadSerializer out to a new class ClaimsSerializer, which allows a subclass to override how the claim is written (needed to handle the aud claim serialization for the payload).

@jimmyjames jimmyjames added this to the v4-Beta milestone Mar 15, 2022
@jimmyjames jimmyjames requested a review from a team as a code owner March 15, 2022 20:00
@Override
public void serialize(T holder, JsonGenerator gen, SerializerProvider provider) throws IOException {
gen.writeStartObject();
holder.getClaims().forEach((key, val) -> System.out.println(key + ": " + val));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we need to remove this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, new commit added to remove

@jimmyjames jimmyjames requested a review from poovamraj March 16, 2022 12:48
Copy link
Copy Markdown
Contributor

@poovamraj poovamraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM!

@jimmyjames jimmyjames merged commit 3b51ee8 into v4-dev Mar 16, 2022
@jimmyjames jimmyjames deleted the ser-header branch March 17, 2022 13:11
@poovamraj poovamraj modified the milestones: v4-Beta, 4.0.0-beta.0 May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup