On Mon, Nov 09, 2015 at 12:44:17PM +0100, Sumit Bose wrote: > On Fri, Nov 06, 2015 at 09:32:23PM +0100, Jakub Hrozek wrote: > > On Fri, Nov 06, 2015 at 09:13:07PM +0100, Jakub Hrozek wrote: > > > On Fri, Nov 06, 2015 at 04:59:58PM +0100, Sumit Bose wrote: > > > > > @@ -658,11 +678,7 @@ static void get_subdomains_callback(struct > > > > > be_req *req, > > > > > */ > > > > > err_maj = dp_err_type; > > > > > err_min = errnum; > > > > > - if (errstr) { > > > > > - err_msg = errstr; > > > > > - } else { > > > > > - err_msg = dp_err_to_string(dp_err_type); > > > > > - } > > > > > + err_msg = safe_be_req_err_msg(errstr, dp_err_type); > > > > > > > > Wouldn't it be even more safe to do this check in > > > > sbus_request_return_and_finish() and not rely on the callers? > > > > > > Something like the attached patch? I think we should check in > > > sbus_request_return_and_finish() in addition to checking in the callers, > > > not instead, because the string in the callback is just a diagnostics > > > message and we can recover by using some fallback. > > > > > > If we get all the way to sbus_request_return_and_finish(), then failing > > > gracefully is better then abort() that libdbus calls, but it's even better > > > to finish the request with wrong error message then fail completely. > > > > > > (I also still dislike that libdbus aborts...) > > > > Also, self-nack. dbus_validate_utf8() is not available on RHEL-6: > > > > http://sssd-ci.duckdns.org/logs/job/32/17/rhel6/ci-build-debug/ci-make-tests.log > > > > I guess we can limit these checks to new OSs or use sss_utf8_check() on > > older platforms.. > > Why not always use sss_utf8_check()? Is there an advantage using > dbus_validate_utf8()?
I don't know how dbus_validate_utf8() works internally, so I tried to stick to the dbus-provided version to check dbus types. But if both do the same thing (If they don't it would be a bug, sure), we can stick to sss_utf8_check() (Another advantage might be that we could get away with linking against libdbus only) _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel