[PATCH] passdb: handle UPN in lookup_name correctly

Andreas Schneider asn at samba.org
Tue Feb 12 09:39:41 UTC 2019


On Monday, February 11, 2019 5:39:33 PM CET Ralph Wuerthner wrote:
> Hi Andreas!
> 
> On 11.02.19 16:07, Andreas Schneider wrote:
> > Hi Ralph,
> > 
> >> Please see attached patchset:
> >> The fix for Samba bugzilla 13312 (commit 1775ac8aa4) caused a regression
> >> when looking up names in UPN notation: Because winbind_lookup_name is
> >> called with lp_workgroup as domain name the lookup is now failing and
> >> the SID for an unmapped Unix user is returned by lookup_name. Fixed by
> >> calling winbind_lookup_name with an empty domain name in case the name
> >> is in UPN notation.
> >> 
> >> The patchset already passed a CI run:
> >> https://gitlab.com/samba-team/devel/samba/pipelines/46689980
> > 
> > Thanks for your contribution!
> > 
> > Please use DGB_DEBUG() instead of DEBUG(10, ...)
> > 
> > In lookup_upn() use a helper variable 'bool ok'. And check talloc_strdup()
> > for NULL.
> 
> Thanks for your feedback! I prepared a new version of the patchset with
> the following changes:
> - using a helper variable in lookup_upn()
> - use DBG_DEBUG() instead of DEBUG(10, ...)
> 
> I didn't add a NULL check for talloc_strdup() because there is already a
> NULL check right after the ok: label. This check is used by other
> sequence steps in lookup_name() too.

But then the function would need documentation that the caller is responsible 
for the NULL check.

I think this is strange code and we should do the check in place and not defer 
it to later.


	Andreas

-- 
Andreas Schneider                      asn at samba.org
Samba Team                             www.samba.org
GPG-ID:     8DFF53E18F2ABC8D8F3C92237EE0FC4DCC014E3D





More information about the samba-technical mailing list