caddyserver / certmagic

Automatic HTTPS for any Go program: fully-managed TLS certificate issuance and renewal
https://pkg.go.dev/github.com/caddyserver/certmagic?tab=doc
Apache License 2.0
5k stars 289 forks source link

Add OCSP stapling unit tests #259

Closed kenjenkins closed 10 months ago

kenjenkins commented 10 months ago

I thought it might be nice to add a few more unit tests for ocsp.go. (This has been at the back of my mind since #245, and I was curious to learn a little bit more about how OCSP works.) Please let me know if you have any concerns, either on the high-level approach or on specific details.

These test cases exercise some more of the logic starting from the stapleOCSP() method, using a few more test certificates and an httptest.Server acting as the OCSP responder.

These tests are very simplistic:

I haven't made any attempt to provide sensible timestamps, either in the test certificates or in the OCSP responses, as it's always a little tricky writing unit tests where time.Now() is involved.

I believe this should bring unit test coverage for ocsp.go from 21% to 70%.

kenjenkins commented 10 months ago

Hi @mholt, please let me know if you have any feedback about this idea. But feel free to close if you think it's not worth bothering with something like this.

mholt commented 10 months ago

Hey @kenjenkins ! So sorry. I am actually very interested in this. We just had a baby a couple months early and she's about to come home finally, so I've been very busy accumulating a backlog of issues and PRs to review.

I've actually been wanting to review this first, since it looks solid and I love tests. I'll do that in a moment. :)

kenjenkins commented 10 months ago

Thanks @mholt, and no need to apologize. Congratulations on your baby girl!