950169 - LogInvalidCertError doesn't use about half of its arguments

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

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

211 void
212 LogInvalidCertError(TransportSecurityInfo *socketInfo, 
213                     const nsACString &host, 
214                     const nsACString &hostWithPort,
215                     int32_t port,
216                     PRErrorCode errorCode,
217                     ::mozilla::psm::SSLErrorMessageType errorMessageType,
218                     nsIX509Cert* ix509)
219 {
220   nsString message;
221   socketInfo->GetErrorLogMessage(errorCode, errorMessageType, message);
222   
223   if (!message.IsEmpty()) {
224     nsContentUtils::LogSimpleConsoleError(message, "SSL");
225   }
226 }

host, hostWithPort, port, and ix509 are unused.

Comment 1

[raymond [:retornam] (inactive)]

Attachment: bug-950169.patch

Comment 2

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

Comment on attachment 8349651 [details] [diff] [review]
bug-950169.patch

Review of attachment 8349651 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. You'll also need to modify the call to LogInvalidCertError around line 423, though.

::: security/manager/ssl/src/SSLServerCertVerification.cpp
@@ +208,5 @@
>  
>  namespace {
>  
>  void
>  LogInvalidCertError(TransportSecurityInfo *socketInfo, 

While we're here, it would be nice to get rid of this unnecessary trailing whitespace.

@@ +215,4 @@
>  {
>    nsString message;
>    socketInfo->GetErrorLogMessage(errorCode, errorMessageType, message);
>    

There's some unnecessary whitespace on this line as well (although we can keep the blank line itself).

Comment 3

[raymond [:retornam] (inactive)]

Attachment: bug-950169.patch

removed the extra whitespaces.

Comment 4

[raymond [:retornam] (inactive)]

Attachment: bug-950169.patch

Comment 5

[raymond [:retornam] (inactive)]

Attachment: bug-950169.patch

Comment 6

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

Comment on attachment 8349716 [details] [diff] [review]
bug-950169.patch

Review of attachment 8349716 [details] [diff] [review]:
-----------------------------------------------------------------

Great!
If you're not already familiar with checkin procedure, I would have a look at this: http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/f95bc5df7fcf

Comment 8

[Phil Ringnalda (:philor)]

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