[PATCH] s3: spoolss: Fix GUID string format on GetPrinter info

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

[PATCH] s3: spoolss: Fix GUID string format on GetPrinter info

Samba - samba-technical mailing list
Hi,

this patch fixes a regression introduced by commit a4157e7c5d75 which
removed the braces around the printer GUID in the printer info level 7
structure, causing AD published printers to be deleted from the
directory by the domain controller's pruning service.

Associated bugzilla entry #12993.

Thanks.

0001-s3-spoolss-Fix-GUID-string-format-on-GetPrinter-info.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter info

Samba - samba-technical mailing list
Hi Samuel,

Your fix looks good, though it'd be nice to have a corresponding unit
test....

On Fri, 22 Sep 2017 15:40:33 +0200, Samuel Cabrero via samba-technical wrote:

> From 41590038643649f50f4a46dee427b9790e3b6c70 Mon Sep 17 00:00:00 2001
> From: Samuel Cabrero <[hidden email]>
> Date: Thu, 21 Sep 2017 09:53:35 +0200
> Subject: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter info
>
> Fix regression introduced by commit a4157e7c5d75 which removed the braces
> around the printer GUID in the printer info level 7 structure.
>
> MS-RPRN section 2.2 says this protocol uses curly-braced GUIDs so printers
> are deleted from the directory by the domain controller's pruning service.
>

Please add a "Bug" tag here, i.e.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12993

> Signed-off-by: Samuel Cabrero <[hidden email]>
Reviewed-by: David Disseldorp <[hidden email]>

Cheers, David

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter info

Samba - samba-technical mailing list
On Wed, Oct 04, 2017 at 03:45:37PM +0200, David Disseldorp via samba-technical wrote:

> Hi Samuel,
>
> Your fix looks good, though it'd be nice to have a corresponding unit
> test....
>
> On Fri, 22 Sep 2017 15:40:33 +0200, Samuel Cabrero via samba-technical wrote:
>
> > From 41590038643649f50f4a46dee427b9790e3b6c70 Mon Sep 17 00:00:00 2001
> > From: Samuel Cabrero <[hidden email]>
> > Date: Thu, 21 Sep 2017 09:53:35 +0200
> > Subject: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter info
> >
> > Fix regression introduced by commit a4157e7c5d75 which removed the braces
> > around the printer GUID in the printer info level 7 structure.
> >
> > MS-RPRN section 2.2 says this protocol uses curly-braced GUIDs so printers
> > are deleted from the directory by the domain controller's pruning service.
> >
>
> Please add a "Bug" tag here, i.e.
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12993
>
> > Signed-off-by: Samuel Cabrero <[hidden email]>
> Reviewed-by: David Disseldorp <[hidden email]>

LGTM. Pushed !

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter info

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
Thanks David.

I extended the rpc.spoolss.printer.addprinter.publish_toggle test to
verify the format.

Cheers

On Wed, 2017-10-04 at 15:45 +0200, David Disseldorp via samba-technical
wrote:

> Hi Samuel,
>
> Your fix looks good, though it'd be nice to have a corresponding unit
> test....
>
> On Fri, 22 Sep 2017 15:40:33 +0200, Samuel Cabrero via samba-
> technical wrote:
>
> > From 41590038643649f50f4a46dee427b9790e3b6c70 Mon Sep 17 00:00:00
> > 2001
> > From: Samuel Cabrero <[hidden email]>
> > Date: Thu, 21 Sep 2017 09:53:35 +0200
> > Subject: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter
> > info
> >
> > Fix regression introduced by commit a4157e7c5d75 which removed the
> > braces
> > around the printer GUID in the printer info level 7 structure.
> >
> > MS-RPRN section 2.2 says this protocol uses curly-braced GUIDs so
> > printers
> > are deleted from the directory by the domain controller's pruning
> > service.
> >
>
> Please add a "Bug" tag here, i.e.
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12993
>
> > Signed-off-by: Samuel Cabrero <[hidden email]>
>
> Reviewed-by: David Disseldorp <[hidden email]>
>
> Cheers, David
>

0001-s3-spoolss-Extend-publish_toggle-test-to-check-retur.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter info

Samba - samba-technical mailing list
On Fri, 06 Oct 2017 10:04:44 +0200, Samuel Cabrero wrote:

> Subject: [PATCH] s3: spoolss: Extend publish_toggle test to check returned
>  GUID string format
>
> Extend the rpc.spoolss.printer.addprinter.publish_toggle test to
> check the format of the returned GUID string in GetPrinter info
> level 7 structure.
>
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12993
>
> Signed-off-by: Samuel Cabrero <[hidden email]>

Looks good, thanks for adding this...
Reviewed-by: David Disseldorp <[hidden email]>

@Jeremy: please push if you're okay with this.

Cheers, David

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter info

Samba - samba-technical mailing list
On Fri, Oct 06, 2017 at 11:09:51AM +0200, David Disseldorp via samba-technical wrote:

> On Fri, 06 Oct 2017 10:04:44 +0200, Samuel Cabrero wrote:
>
> > Subject: [PATCH] s3: spoolss: Extend publish_toggle test to check returned
> >  GUID string format
> >
> > Extend the rpc.spoolss.printer.addprinter.publish_toggle test to
> > check the format of the returned GUID string in GetPrinter info
> > level 7 structure.
> >
> > Bug: https://bugzilla.samba.org/show_bug.cgi?id=12993
> >
> > Signed-off-by: Samuel Cabrero <[hidden email]>
>
> Looks good, thanks for adding this...
> Reviewed-by: David Disseldorp <[hidden email]>
>
> @Jeremy: please push if you're okay with this.

Can you add missing return NULL from allocation
checks in:

+               /* Build reference GUID string */
+               ref_guid = GUID_string2(tctx, &guid);
+               ref_guid = talloc_strdup_upper(tctx, ref_guid);

please ?

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter info

Samba - samba-technical mailing list
Hi Jeremy,

checks added.

Cheers.

On Fri, 2017-10-06 at 09:59 -0700, Jeremy Allison via samba-technical
wrote:

> On Fri, Oct 06, 2017 at 11:09:51AM +0200, David Disseldorp via samba-
> technical wrote:
> > On Fri, 06 Oct 2017 10:04:44 +0200, Samuel Cabrero wrote:
> >
> > > Subject: [PATCH] s3: spoolss: Extend publish_toggle test to check
> > > returned
> > >  GUID string format
> > >
> > > Extend the rpc.spoolss.printer.addprinter.publish_toggle test to
> > > check the format of the returned GUID string in GetPrinter info
> > > level 7 structure.
> > >
> > > Bug: https://bugzilla.samba.org/show_bug.cgi?id=12993
> > >
> > > Signed-off-by: Samuel Cabrero <[hidden email]>
> >
> > Looks good, thanks for adding this...
> > Reviewed-by: David Disseldorp <[hidden email]>
> >
> > @Jeremy: please push if you're okay with this.
>
> Can you add missing return NULL from allocation
> checks in:
>
> +               /* Build reference GUID string */
> +               ref_guid = GUID_string2(tctx, &guid);
> +               ref_guid = talloc_strdup_upper(tctx, ref_guid);
>
> please ?
>

0001-s3-spoolss-Extend-publish_toggle-test-to-check-retur.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter info

Samba - samba-technical mailing list
On Mon, Oct 09, 2017 at 12:32:40PM +0200, Samuel Cabrero wrote:
> Hi Jeremy,
>
> checks added.

LGTM. RB+. Can I get a second Team reviewer ?

> On Fri, 2017-10-06 at 09:59 -0700, Jeremy Allison via samba-technical
> wrote:
> > On Fri, Oct 06, 2017 at 11:09:51AM +0200, David Disseldorp via samba-
> > technical wrote:
> > > On Fri, 06 Oct 2017 10:04:44 +0200, Samuel Cabrero wrote:
> > >
> > > > Subject: [PATCH] s3: spoolss: Extend publish_toggle test to check
> > > > returned
> > > >  GUID string format
> > > >
> > > > Extend the rpc.spoolss.printer.addprinter.publish_toggle test to
> > > > check the format of the returned GUID string in GetPrinter info
> > > > level 7 structure.
> > > >
> > > > Bug: https://bugzilla.samba.org/show_bug.cgi?id=12993
> > > >
> > > > Signed-off-by: Samuel Cabrero <[hidden email]>
> > >
> > > Looks good, thanks for adding this...
> > > Reviewed-by: David Disseldorp <[hidden email]>
> > >
> > > @Jeremy: please push if you're okay with this.
> >
> > Can you add missing return NULL from allocation
> > checks in:
> >
> > +               /* Build reference GUID string */
> > +               ref_guid = GUID_string2(tctx, &guid);
> > +               ref_guid = talloc_strdup_upper(tctx, ref_guid);
> >
> > please ?
> >

> From 9b9c2c63b968d878bebe7e4854046aef89ec357f Mon Sep 17 00:00:00 2001
> From: Samuel Cabrero <[hidden email]>
> Date: Thu, 5 Oct 2017 19:22:29 +0200
> Subject: [PATCH] s3: spoolss: Extend publish_toggle test to check returned
>  GUID string format
>
> Extend the rpc.spoolss.printer.addprinter.publish_toggle test to
> check the format of the returned GUID string in GetPrinter info
> level 7 structure.
>
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12993
>
> Signed-off-by: Samuel Cabrero <[hidden email]>
> ---
>  source4/torture/rpc/spoolss.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c
> index d4f69698d96..31b9525ee62 100644
> --- a/source4/torture/rpc/spoolss.c
> +++ b/source4/torture/rpc/spoolss.c
> @@ -9218,14 +9218,26 @@ static bool test_printer_set_publish(struct torture_context *tctx,
>   "info7 publish flag not set");
>   } else {
>   struct GUID guid;
> + char *ref_guid;
>   torture_assert_int_equal(tctx,
>   info.info7.action,
>   DSPRINT_PUBLISH,
>   "info7 publish flag not set");
> +
> + /* GUID_from_string is able to parse both plain and
> + * curly-braced guids */
>   torture_assert_ntstatus_ok(tctx,
>     GUID_from_string(info.info7.guid,
>     &guid),
>     "invalid published printer GUID");
> +
> + /* Build reference GUID string */
> + ref_guid = GUID_string2(tctx, &guid);
> + torture_assert_not_null(tctx, ref_guid, "ENOMEM");
> + ref_guid = talloc_strdup_upper(tctx, ref_guid);
> + torture_assert_not_null(tctx, ref_guid, "ENOMEM");
> + torture_assert_str_equal(tctx, info.info7.guid, ref_guid,
> + "invalid GUID format");
>   }
>  
>   return true;
> --
> 2.14.2
>


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] s3: spoolss: Fix GUID string format on GetPrinter info

Samba - samba-technical mailing list
On Mon, 9 Oct 2017 15:01:02 -0700, Jeremy Allison wrote:

> > checks added.  
>
> LGTM. RB+. Can I get a second Team reviewer ?

RB+. Pushed to autobuild.

Cheers, David