1004154 - Move/reimplement more cleanly OCSP fetching code from nsNSSCallbacks.cpp to OCSPRequestor.cpp

Status: RESOLVED DUPLICATE of bug 1456489

(Core :: Security: PSM, defect, P3)

Description

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO? for response) from bug 982248 comment #11)
> It would be great if we could "hg cp ../manager/ssl/src/nsNSSCallbacks.cpp
> OCSPRequester.cpp" to move the OCSP callback code to OCSPRequester.cpp.
> Could be done later too though; if so, please file the follow-up bug for
> doing it.

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

(More specifically, the nsNSSHttpInterface parts of nsNSSCallbacks.cpp need to move.)

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

This is basically just moving the code between "class nsHTTPDownloadEvent" and "nsHTTPListener::send_done_signal", inclusive, to OCSPRequestor.cpp, minimizing the #includes in nsNSSCallbacks.cpp, and making sure it compiles and that the tests pass ("mach xpcshell-test security/manager/ssl/tests/unit" in particular).

Additionally, in a separate patch, the code could be simplified to avoid the use of NSS's SEC_HttpClientFcnV1 completely. The code only currently uses that because we used to use NSS's ocsp.c OCSP stuff, which required it. Now it is superfluous.

Comment 3

[:Cykesiopka]

(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #2)
> Additionally, in a separate patch, the code could be simplified to avoid the
> use of NSS's SEC_HttpClientFcnV1 completely. The code only currently uses
> that because we used to use NSS's ocsp.c OCSP stuff, which required it. Now
> it is superfluous.

This ended up being done in Bug 1004149.

Comment 4

[:Cykesiopka]

I'm morphing this bug a bit since IMO it makes sense to make at least some API improvements when moving the code. See Bug 1004149 comment 7 in particular for issues with the current API.

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

A lot of improvements were made in bug 1456489, although the implementation is still in nsNSSCallbacks.cpp. I'm not sure it's important to move it at this point.