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

Preserve XSD dateTime and dateTimeStamp lexical representations. #639

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

timothee-haudebourg
Copy link
Contributor

@timothee-haudebourg timothee-haudebourg commented Dec 19, 2024

Description

Adds a Lexical<T> wrapper type that preserves the lexical representation of xsd_types::DateTime and xsd_types::DateTimeStamp.

Also bumps json-ld to version 0.21.2, making sure that typed string literals are not canonicalized.

Other changes

Pinned Rust version to 1.81.0 in the CI, to avoid clippy warnings.

Tested

  • Unit tests for Lexical<T>.
  • Test suite.

@timothee-haudebourg timothee-haudebourg marked this pull request as draft December 19, 2024 13:43
@timothee-haudebourg timothee-haudebourg marked this pull request as ready for review December 19, 2024 14:11
@timothee-haudebourg
Copy link
Contributor Author

I had to disable the vcdm_v2_sign::ecdsa_secp256r1_2020 test for unrelated reasons (it's using either the wrong cryptosuite or the wrong vcdm version, not sure why we didn't detect the error sooner).

@timothee-haudebourg timothee-haudebourg force-pushed the timothee-haudebourg/issue638 branch from 9af0eee to ae0e95b Compare December 20, 2024 10:08
Copy link
Member

@sbihel sbihel left a comment

Choose a reason for hiding this comment

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

I'm approving to unblock other work, but the failure of ecdsa_secp256r1_2020 is worrying.

It looks like just adding https://w3id.org/security/v2 to the context fixes the test, but something larger seems to be at play. I would suggest uncommenting the test with this fix, and have follow-ups for:

  • addressing the Clippy warnings from Rust 1.82
  • investigating this context issue, and either have a fix or determine if it's not a big deal as it's an old cryptosuite

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.

2 participants