Skip to content

CertContext: method to retrieve time when added to store#7

Merged
ancwrd1 merged 1 commit intorustls:devfrom
BiagioFesta:wip/bfesta/added_time
Sep 10, 2025
Merged

CertContext: method to retrieve time when added to store#7
ancwrd1 merged 1 commit intorustls:devfrom
BiagioFesta:wip/bfesta/added_time

Conversation

@BiagioFesta
Copy link
Contributor

@BiagioFesta BiagioFesta commented Sep 9, 2025

If out of the scope of this crate, feel free to close this PR

@BiagioFesta BiagioFesta force-pushed the wip/bfesta/added_time branch from a324fe0 to 1694326 Compare September 9, 2025 17:02
src/cert.rs Outdated
/// Returns the time when the certificate was added to the store.
///
/// `None` if the property is not set.
pub fn property_date_stamp(&self) -> Result<Option<time::UtcDateTime>> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can it be unit-tested, e.g. with the pkcs12 file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure, aren't those properties part of the context of the win store?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know, that's why I am asking :) But AI says that it's specific to the certstore indeed. A possible unit test could be to create a temporary entry in the user store, and then delete it after test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this would go beyond the scope of a Unit Test maybe? it's more like an integration test considering the impact on the external system dependency.

Also take into account that's not trivial: the library does not allow to set the property (yet) and creation of store, build, and add certificate

Copy link
Collaborator

Choose a reason for hiding this comment

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

But if it was manually tested, I think it's enough for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, let's skip the test for it.

Copy link
Contributor Author

@BiagioFesta BiagioFesta Sep 10, 2025

Choose a reason for hiding this comment

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

To be honest, I am not a Windows guy :)
I use Linux, and develop on Linux.

But I made a check on a Windows laptop in order to develop the feature, and when the property was set on a certificate store of mine, I get the info as expected

@ancwrd1 ancwrd1 merged commit 69b7699 into rustls:dev Sep 10, 2025
2 checks passed
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