[PATCH] Fix smbclient regression not printing session setup anymore

[Jeremy Allison]

On Tue, Jun 06, 2017 at 06:47:20PM +0200, Andreas Schneider wrote:
> On Tuesday, 6 June 2017 18:29:59 CEST Jeremy Allison wrote:
> > On Tue, Jun 06, 2017 at 06:27:00PM +0200, Andreas Schneider via samba-
> technical wrote:
> > > On Tuesday, 6 June 2017 18:08:09 CEST Jeremy Allison wrote:
> > > > On Tue, Jun 06, 2017 at 05:56:21PM +0200, Andreas Schneider via samba-
> > > 
> > > technical wrote:
> > > > > On Tuesday, 6 June 2017 17:35:52 CEST Andreas Schneider via
> > > > > samba-technical
> > > > > 
> > > > > wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > with Samba 4.6 we have a regression. We do not get the information
> > > > > > of
> > > > > > the
> > > > > > session setup filled out so smbclient can't print it.
> > > > > > 
> > > > > > Domain=[SAMBA-TEST] OS=[Windows 6.1] Server=[Samba
> > > > > > 4.7.0pre1-DEVELOPERBUILD] smb: \>
> > > > > > 
> > > > > > The attached patchset addresses the issue. I've open a bug for this:
> > > > > > 
> > > > > > https://bugzilla.samba.org/show_bug.cgi?id=12824
> > > > > 
> > > > > I've added a test to confirm that the message is actually printed so
> > > > > we do
> > > > > not regress again ...
> > > > 
> > > > The change from 'bool align_odd = true' to 'bool align_odd = false'
> > > > concerns me.
> > > > 
> > > > smb_bytes_pull_str() is used from all SMB1 session code inside
> > > > libcli/smb/smb1cli_session.c
> > > 
> > > The code called smb_bytes_talloc_string() before and this didn't handle
> > > align_odd before, why should it now?
> > 
> > Sorry, I'm being dumb. What called smb_bytes_talloc_string() before ?
> 
> 9fffec88033a385f3ebb8fe8520b9b39c831d98f
> 
> and
> 
> a54d250e0962e1c3777653af5c6f7b7c6eb04d24
> 
> are the changes to call the new functions. The code called 
> smb_bytes_talloc_string() before.
> 
> > 
> > > If you set align_odd to true you get:
> > > 
> > > OS=[椀渀搀漀眀猀 㘀⸀㄀] Server=[匀愀洀戀愀 㐀⸀㜀⸀　瀀爀攀㄀ⴀ䐀䔀嘀䔀䰀伀倀䔀刀䈀唀䤀䰀䐀]
> > 
> > I'm not saying you're wrong, I just want to know you thought
> > about all the implications :-) :-).
> 
> It's always used to pull the same strings. So if one of these calls is wrong 
> we will see crippled strings in the smbclient output again ...

OK, thanks for going through it with me Andreas.

smb_bytes_pull_str() and the 'bool align_odd' parameter
is a completely redundent wrapper around internal_bytes_pull_str(),
which seems to have been added to make it look similar
to internal_bytes_push_str() - so the 'bool align_odd' parameter
isn't actually needed at all, but that's a patch for another
day :-).

RB+.

Cheers,

	Jeremy.