[PATCH] vfs_fruit checks wrong AAPL config state and so always uses readdirattr

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] vfs_fruit checks wrong AAPL config state and so always uses readdirattr

Ralph Böhme-2
Hi!

Attached is a simple fix for this bug:
<https://bugzilla.samba.org/show_bug.cgi?id=12541>

Kudos to Shilpa Krishnareddy who spotted this!

Please review & push if happy. Thanks!

Cheerio!
-slow

master-bug12541-config-aapl.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] vfs_fruit checks wrong AAPL config state and so always uses readdirattr

Jeremy Allison
On Thu, Jan 26, 2017 at 12:40:26PM +0100, Ralph Böhme wrote:
> Hi!
>
> Attached is a simple fix for this bug:
> <https://bugzilla.samba.org/show_bug.cgi?id=12541>
>
> Kudos to Shilpa Krishnareddy who spotted this!
>
> Please review & push if happy. Thanks!

LGTM. Obvious fix ! Pushed.

Thanks,

Jeremy.

> From 5f78c4cd7d959ad9208cefd15641d89efd1513b8 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <[hidden email]>
> Date: Thu, 26 Jan 2017 11:49:55 +0100
> Subject: [PATCH] vfs_fruit: checks wrong AAPL config state and so always uses
>  readdirattr
>
> readdirattr should only be enabled if the client enables it via AAPL
> negotitiation, not for all clients when vfs_fruit is loaded.
>
> Unfortunately the check in fruit_readdir_attr() is
>
>   if (!config->use_aapl) {
>     return SMB_VFS_NEXT_READDIR_ATTR(handle, fname, mem_ctx, pattr_data);
>   }
>
> This uses the wrong config state "use_aapl" which is always true by
> default (config option "fruit:aapl").
>
> We must use "nego_aapl" instead which is only true if the client
> really negotiated this feature.
>
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12541
>
> Signed-off-by: Ralph Boehme <[hidden email]>
> ---
>  source3/modules/vfs_fruit.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index c46fdbf..028ea0d 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -4992,7 +4992,7 @@ static NTSTATUS fruit_readdir_attr(struct vfs_handle_struct *handle,
>   struct fruit_config_data,
>   return NT_STATUS_UNSUCCESSFUL);
>  
> - if (!config->use_aapl) {
> + if (!config->nego_aapl) {
>   return SMB_VFS_NEXT_READDIR_ATTR(handle, fname, mem_ctx, pattr_data);
>   }
>  
> --
> 2.7.4
>


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] vfs_fruit checks wrong AAPL config state and so always uses readdirattr

Uri Simchoni-4
In reply to this post by Ralph Böhme-2
On 01/26/2017 01:40 PM, Ralph Böhme wrote:

> Hi!
>
> Attached is a simple fix for this bug:
> <https://bugzilla.samba.org/show_bug.cgi?id=12541>
>
> Kudos to Shilpa Krishnareddy who spotted this!
>
> Please review & push if happy. Thanks!
>
> Cheerio!
> -slow
>
Question (chiming in VERY late...):
Since Macs only enable AAPL once per SMB connection (or session, not
sure which one..), won't this break AAPL when a new tree-connect is
made? (+ the copyfile and POSIX rename stuff)

Thanks,
Uri

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] vfs_fruit checks wrong AAPL config state and so always uses readdirattr

Uri Simchoni-4
On 02/28/2017 08:08 AM, Uri Simchoni wrote:

> On 01/26/2017 01:40 PM, Ralph Böhme wrote:
>> Hi!
>>
>> Attached is a simple fix for this bug:
>> <https://bugzilla.samba.org/show_bug.cgi?id=12541>
>>
>> Kudos to Shilpa Krishnareddy who spotted this!
>>
>> Please review & push if happy. Thanks!
>>
>> Cheerio!
>> -slow
>>
> Question (chiming in VERY late...):
> Since Macs only enable AAPL once per SMB connection (or session, not
> sure which one..), won't this break AAPL when a new tree-connect is
> made? (+ the copyfile and POSIX rename stuff)
>
> Thanks,
> Uri
>
I mean, of course, that an additional fix may be required, to persist
neg_aapl across <whatever it should be persistent against>, not that
*this* fix is bad.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] vfs_fruit checks wrong AAPL config state and so always uses readdirattr

Ralph Böhme-2
On Tue, Feb 28, 2017 at 08:20:12AM +0200, Uri Simchoni wrote:

> On 02/28/2017 08:08 AM, Uri Simchoni wrote:
> > On 01/26/2017 01:40 PM, Ralph Böhme wrote:
> >> Hi!
> >>
> >> Attached is a simple fix for this bug:
> >> <https://bugzilla.samba.org/show_bug.cgi?id=12541>
> >>
> >> Kudos to Shilpa Krishnareddy who spotted this!
> >>
> >> Please review & push if happy. Thanks!
> >>
> >> Cheerio!
> >> -slow
> >>
> > Question (chiming in VERY late...):
> > Since Macs only enable AAPL once per SMB connection (or session, not
> > sure which one..), won't this break AAPL when a new tree-connect is
> > made? (+ the copyfile and POSIX rename stuff)
> >
> > Thanks,
> > Uri
> >
> I mean, of course, that an additional fix may be required, to persist
> neg_aapl across <whatever it should be persistent against>, not that
> *this* fix is bad.

Oh, thanks a lot for spotting this! I've created bug #12604 for this and will
prepare a patch later on.

Cheerio!
-slow