1749017 - client cert dialog is no longer displaying dates in validity period

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Julien Cristau [:jcristau]]

As far as I can tell https://hg.mozilla.org/mozilla-central/diff/b7eb41176fd5a043e5b9eff0396761c5a8da8320/security/manager/ssl/X509CertValidity.cpp changed the date format from "long" to "nothing".

Comment 1

[Julien Cristau [:jcristau]]

Attachment: Bug 1749017 - bring the date back in X509CertValidity::FormatTime. r?keeler,gregtatum

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1715892

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8b97bc59958e
bring the date back in X509CertValidity::FormatTime. r=gregtatum,keeler

Comment 4

[Cristian Tuns]

Backed out for causing xpcshell failures on test_nsIX509CertValidity.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | security/manager/ssl/tests/unit/test_nsIX509CertValidity.js | fuzzyEqual - [fuzzyEqual : 11] Parse "January 1, 2013 at 12:00:00 AM GMT" as a date - false == true

Comment 5

[Julien Cristau [:jcristau]]

So the test passes on linux, with "January 1, 2013, 01:00:00 GMT+1" as the notBefore. On mac the string returned as cert.validity.notBeforeLocalTime is "January 1, 2013 at 12:00:00 AM GMT" instead and Date.parse doesn't like that and returns NaN. Any suggestion, Greg?

Comment 6

[Greg Tatum [:gregtatum]]

I would recommend not using Date.parse for the check here as the localized Date string should be treated as an opaque structure. Generally localizers are allowed to do whatever they want with the string, and the developer settings are a request for it to be treated that way. Here it looks like the time zone is different between the two dates.

Keep in mind that formatted dates can vary +/- 14 hours (and in non-hour increments) depending on the time zone of the machine running the code. Also the code can break when localizers change how dates are formatted.

So January 1st at midnight is a tough date to rely on, as it could be January 1st of 2013, OR Dec 31st of 2012 depending on the machine.

If you need to do diffs of time, then it would be good to store a numeric Date object that contains the information you need. It's probably safe to do some light regex checking of a date if you keep in mind the +/- 14 hour time zone issue.

When I've written tests to check if I get a formatted date, then I generally write some permissive regexes that check for specific date components, like the year or time. For instance you can check for the \b201[23]\b to ensure the year component is there, and a \b\d+:\d+\b for the time component. Those will most likely be future proof.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df6cc4a54bc9
bring the date back in X509CertValidity::FormatTime. r=gregtatum,keeler

Comment 8

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/df6cc4a54bc9

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jcristau, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 10

[Julien Cristau [:jcristau]]

Comment on attachment 9258049 [details]
Bug 1749017 - bring the date back in X509CertValidity::FormatTime. r?keeler,gregtatum

Beta/Release Uplift Approval Request

• User impact if declined: the client certificate dialogs are missing validity dates; regression introduced in 96.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: there's a test but it doesn't catch this issue :(
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): fairly trivial patch
• String changes made/needed:

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9258049 [details]
Bug 1749017 - bring the date back in X509CertValidity::FormatTime. r?keeler,gregtatum

Approved for 97.0b8.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a311819da945