[PATCH 5/6] s3-spoolss: Support opening ports via OpenPrinterEx

[Justin Chevrier]

On Sun, Oct 30, 2011 at 9:12 PM, David Disseldorp <ddiss at suse.de> wrote:
> On Wed, 31 Aug 2011 15:03:13 -0400
> Justin Chevrier <jchevrier at gmail.com> wrote:
>
> ...
>>  static WERROR set_printer_hnd_name(TALLOC_CTX *mem_ctx,
> ...
>> +                     /* Ensure there isn't a second backslash */
>> +                     if ( *aprinter == '\\' ) {
>> +                             *aprinter = '\0';
>> +                             aprinter++;
>> +                     }
>
> Is this needed, I.e. have you seen Windows clients send
> \\server\\printer UNCs?
Yes, I did get Windows XP SP3 to send a request in this format. It
appeared when bringing up the properties of the port I believe. I'm
pretty sure I have a Wireshark capture of the request somewhere around
here. I'll see if I can dig it up.
>
>>               }
>>               if (!is_myname_or_ipaddr(servername)) {
>>                       return WERR_INVALID_PRINTER_NAME;
>> @@ -627,7 +634,36 @@ static WERROR set_printer_hnd_name(TALLOC_CTX *mem_ctx,
>>               fstrcpy(sname, SPL_XCV_MONITOR_LOCALMON);
>>               found = true;
>>       }
>> +     else if ( strstr(aprinter, SPL_XCV_PORT) ) {
>> +             if ((portname = strchr_m(aprinter, ' ')) != NULL ) {
>> +                     portname++;
>> +                     /* Don't bother searching the registry for
>> +                        dummy port name: "Samba Printer Port" */
>> +                     if (!strequal(portname, SAMBA_PRINTER_PORT_NAME)) {
>> +
>> +                             result = winreg_get_port_internal(mem_ctx,
>> +                                                               session_info,
>> +                                                               msg_ctx,
>> +                                                               portname,
>> +                                                               &data1);
>> +                             if (!W_ERROR_IS_OK(result)) {
>> +                                     DEBUG(2,("set_printer_hnd_name: failed to lookup port [%s] -- result [%s]\n",
>> +                                             portname, win_errstr(result)));
>> +                                     return WERR_INVALID_PRINTER_NAME;
>> +                             }
>>
>> +                             DEBUG(4, ("Found Port: [%s]\n", portname));
>> +                             fstrcpy(sname, portname);
>> +                             Printer->printer_type = SPLHND_PORTMON_TCP;
>> +
>> +                             TALLOC_FREE(data1);
>> +                             goto done;
>> +                     }
>> +             }
>> +
>> +             DEBUGADD(4,("Port %s not found\n", portname));
>> +             return WERR_INVALID_PRINTER_NAME;
>> +     }
>
> This doesn't appear right to me. If I understand this correctly lookup of
> the printer name is skipped if the port is found. Also, in the case of
> the default printer port (SAMBA_PRINTER_PORT_NAME) an error is returned
> immediately.
>
This code path is only entered if the client issues an OpenPrinterEx
against a port (basically ',XcvPort <portname>'); all other
OpenPrinterEx requests against actual printers will skip right past
this code. Or maybe I haven't understood the question. In terms of an
error returned on SAMBA_PRINTER_PORT_NAME, I structured it like that
to mirror what happens in current code (all OpenPrinterEx calls
against ports fail). Basically if the URI in CUPS isn't a network
based printer the code sets the port name associated with the printer
to: SAMBA_PRINTER_PORT_NAME and all attempts to issue an OpenPrinterEx
against that port fail as it isn't a specific port, just a catch all.
Going forward it would likely make more sense to associate local or
other URIs against equivalents in Windows, it was just beyond the
scope of this work. Again, apologies if I've misunderstood.