Re: [PATCH] Logging of honored certificate validation error names

On 11/05/2011 04:15 PM, Amos Jeffries wrote:
> On 2/11/2011 11:13 p.m., Tsantilas Christos wrote:
>> Currently the %err_detail access_log formating code does not display
>> something useful for the system admin in the case of the certificate
>> validation errors.
>>
>> This patch in the case of an ERR_SECURE_CONNECT_FAIL error displays
>> the certificate validation error name.
>>
>
> +1. Looks okay.
>
> I'm a little dubious about passing request->detailError() the SSL error
> code instead of the errno. But have no strong objections.

Error detailing code was specifically designed to record
context-specific details beyond errno (which was already available in
most cases) and the request->detailError() method itself is usually used
to store non-errno details:

> ./src/Server.cc: request->detailError(ERR_ICAP_FAILURE, ERR_DETAIL_RESPMOD_BLOCK_LATE);
> ./src/client_side_request.cc: request->detailError(ERR_ACCESS_DENIED, ERR_DETAIL_REQMOD_BLOCK);
> ./src/client_side_request.cc: request->detailError(ERR_ICAP_FAILURE, ERR_DETAIL_CLT_REQMOD_RESP_BODY);
> ./src/client_side_request.cc: request->detailError(ERR_ICAP_FAILURE, errDetail);

We even document HttpRequest::errDetail to be errType-specific:

> err_type errType;
> int errDetail; ///< errType-specific detail about the transaction error

Why are you dubious about passing request->detailError() the SSL error code?

Thank you,

Alex.
Received on Mon Nov 07 2011 - 20:49:00 MST