On Fri, Nov 13, 2015 at 01:40:16PM +0100, Sumit Bose wrote:
> Hi,
>
> attached are patches that only use sss_utf8_check() and work on RHEL-6
> also.
The patches look good and fixing the issue as expected. CI passes as
well
http://sssd-ci.duckdns.org/logs/job/33/16/summary.html (the RHEL6
failure is unrelated). ACK
I just have a minor comment
>
> +static const char *safe_be_req_err_msg(const char *msg_in,
> + int dp_err_type)
> +{
> + bool ok;
> +
> + if (msg_in == NULL) {
> + /* No custom error, just use default */
> + return dp_err_to_string(dp_err_type);
> + }
> +
> + ok = sss_utf8_check((const uint8_t *) msg_in,
> + strlen(msg_in));
> + if (!ok) {
> + DEBUG(SSSDBG_MINOR_FAILURE,
> + "Back end message is invalid, using default\n");
Maybe something like
DEBUG(SSSDBG_MINOR_FAILURE,
"Back end message [%s] contains invalid non-UTF8 character, " \
"using default\n", msg_in);
would be more specific and would help to better identify the source of the
message as well? Similar for sbus_request_valist_check().
But to fix the issue at hand and not to delay the release this can of
course be added later as well.
No, I think it's important to have good debug messages. I used the one
you suggested, there are no other changes in the patches.