pycups patch for writeRequestData and binary files

[Doug Reynolds]

Hello all,
For pycups method, cups.writeRequestData, there is a bug that will corrupt binary data
when streamed through this method.
This python code will expose the problem:
import cups
x = None

conn = cups.Connection()

id = conn.createJob('printer', 'blah', { 'document-format':
cups.CUPS_FORMAT_AUTO })
conn.startDocument('printer', id, 'asdfasd', cups.CUPS_FORMAT_AUTO, 1)

with open('test.pdf', 'rb') as f:
    while True:
        x = f.read(1024)
        if len(x):
            conn.writeRequestData(x, len(x))
        else:
            break

conn.finishDocument('printer')

The printer will print out a sheet with the text:
PDF Error 10: PostScript: Error: IOERRORAssert FaileD: rc == curStrLen

If you take the print queue down and print the same file with lpr -P printer, the md5sums
in /var/spool/cups are different.  Each lpr command produces a queued file with the same
hash as the original file.  Each file printed through the API will have a different sum
each time.
The issue is caused by this line in the code:
buffer = strdup(PyBytes_AsString (buffer_obj));
The strdup detects a null in the Python byte array and ends the copy.  This causes a
buffer overrun when "status = cupsWriteRequestData(self->http, buffer,
length);" is called.  This is because the buffer might only have 120 bytes in it, and
the length is equal to, says, 8192.  

I switched over to memcpy, and malloc'd a separate buffer.  I choose memcpy as this
method is not overrun safe.  The caller must mind his or her own buffer size.  The method
now works with jpgs, pdfs, and regular text files.
Feel free to make my code better as my C is a bit rusty.
Doug

[Jiri Popelka]

Your patch looks good to me and has been merged.

Thank you !

--
Jiri

On 24.3.2016 18:31, reynoldsdo(a)yahoo.com wrote:
>  Hello all,
>
>  For pycups method, cups.writeRequestData, there is a bug that will
>  corrupt binary data when streamed through this method.
>
>  This python code will expose the problem:
>
>  import cups
>  x = None
>
>  conn = cups.Connection()
>
>  id = conn.createJob('printer', 'blah', { 'document-format':
>  cups.CUPS_FORMAT_AUTO })
>  conn.startDocument('printer', id, 'asdfasd', cups.CUPS_FORMAT_AUTO, 1)
>
>  with open('test.pdf', 'rb') as f:
>      while True:
>          x = f.read(1024)
>          if len(x):
>              conn.writeRequestData(x, len(x))
>          else:
>              break
>
>  conn.finishDocument('printer')
>
>  The printer will print out a sheet with the text:
>
>  PDF Error 10: PostScript: Error: IOERROR
>  Assert FaileD: rc == curStrLen
>
>  If you take the print queue down and print the same file with lpr -P
>  printer, the md5sums in /var/spool/cups are different.  Each lpr
>  command produces a queued file with the same hash as the original
>  file.  Each file printed through the API will have a different sum
>  each time.
>
>  The issue is caused by this line in the code:
>
>  buffer = strdup(PyBytes_AsString (buffer_obj));
>
>  The strdup detects a null in the Python byte array and ends the copy. 
>  This causes a buffer overrun when "status =
>  cupsWriteRequestData(self->http, buffer, length);" is called.  This is
>  because the buffer might only have 120 bytes in it, and the length is
>  equal to, says, 8192. 
>
>  I switched over to memcpy, and malloc'd a separate buffer.  I choose
>  memcpy as this method is not overrun safe.  The caller must mind his
>  or her own buffer size.  The method now works with jpgs, pdfs, and
>  regular text files.
>
>  Feel free to make my code better as my C is a bit rusty.
>
>  Doug

[Doug Reynolds]

Thank you for merging the patch and maintaining the API.

[Francesco Faraone]

Hi everybody,

I've installed pycups 1.9.73 and in cupsconnection.c I can see:

  /*
   * Assign result to buffer
   */

  buffer = PyBytes_AsString (buffer_obj);

  /*
   * Allocate memory and duplicate buffer
   */

  data = (char *) malloc((size_t) length);

  if (data == NULL)
    return NULL;

  memcpy (data, buffer, length);

  debugprintf ("-> Connection_writeRequestData(length=%d)\n", length);

  Connection_begin_allow_threads (self);
  status = cupsWriteRequestData(self->http, data, length);
  Connection_end_allow_threads (self);



So the writeRequestData method use memcpy as the patch that has been merged.
But unfortunately the patch doesn't work for me. I have made a test with the same code
snippet of this thread with another pdf file.

I can see that the spooled document differs from the original one.


Could you please help me ?

Thanks

Francesco

[Francesco Faraone]

Hi

I have changed the buffer_obj type from PyObject to PyByteArrayObject and now it works.

static PyObject *
Connection_writeRequestData (Connection *self, PyObject *args, PyObject *kwds)
{
  static char *kwlist[] = { "buffer", "length", NULL };
  PyByteArrayObject *buffer_obj;
  char *buffer;             /* string from python */
  char *data;               /* data to send */
  int length;
  http_status_t	status;		/* Write status */

  if (!PyArg_ParseTupleAndKeywords (args, kwds, "Oi", kwlist,
				    &buffer_obj, &length))
    return NULL;

  /*
   * Assign result to buffer
   */

  buffer = PyBytes_AsString ((PyObject*)buffer_obj);

  /*
   * Allocate memory and duplicate buffer
   */

  data = (char *) malloc((size_t) length);

  if (data == NULL)
    return NULL;

  memcpy (data, buffer, length);

  debugprintf ("-> Connection_writeRequestData(length=%d)\n", length);

  Connection_begin_allow_threads (self);
  status = cupsWriteRequestData(self->http, data, length);
  Connection_end_allow_threads (self);

  if (status != HTTP_CONTINUE)
  {
    free (data);
    set_ipp_error (cupsLastError (), cupsLastErrorString ());
    debugprintf ("<- Connection_writeRequestData() = NULL\n");
    return NULL;
  }

  free (data);
  debugprintf ("<- Connection_writeRequestData() = %d\n", status);
#if PY_MAJOR_VERSION >= 3
  return PyLong_FromLong (status);
#else
  return PyInt_FromLong (status);
#endif
}

I hope this change is good for you, I have a poor experience with python c binding.

Thanks

Francesco

[Francesco Faraone]

here is the path

On Thu, Jun 23, 2016 at 12:50 PM, <ffaraone(a)gaa9.com&gt; wrote:

>  Hi
>
>  I have changed the buffer_obj type from PyObject to PyByteArrayObject and
>  now it works.
>
>  static PyObject *
>  Connection_writeRequestData (Connection *self, PyObject *args, PyObject
>  *kwds)
>  {
>    static char *kwlist[] = { "buffer", "length", NULL };
>    PyByteArrayObject *buffer_obj;
>    char *buffer;             /* string from python */
>    char *data;               /* data to send */
>    int length;
>    http_status_t status;         /* Write status */
>
>    if (!PyArg_ParseTupleAndKeywords (args, kwds, "Oi", kwlist,
>                                      &buffer_obj, &length))
>      return NULL;
>
>    /*
>     * Assign result to buffer
>     */
>
>    buffer = PyBytes_AsString ((PyObject*)buffer_obj);
>
>    /*
>     * Allocate memory and duplicate buffer
>     */
>
>    data = (char *) malloc((size_t) length);
>
>    if (data == NULL)
>      return NULL;
>
>    memcpy (data, buffer, length);
>
>    debugprintf ("-> Connection_writeRequestData(length=%d)\n", length);
>
>    Connection_begin_allow_threads (self);
>    status = cupsWriteRequestData(self->http, data, length);
>    Connection_end_allow_threads (self);
>
>    if (status != HTTP_CONTINUE)
>    {
>      free (data);
>      set_ipp_error (cupsLastError (), cupsLastErrorString ());
>      debugprintf ("<- Connection_writeRequestData() = NULL\n");
>      return NULL;
>    }
>
>    free (data);
>    debugprintf ("<- Connection_writeRequestData() = %d\n", status);
>  #if PY_MAJOR_VERSION >= 3
>    return PyLong_FromLong (status);
>  #else
>    return PyInt_FromLong (status);
>  #endif
>  }
>
>  I hope this change is good for you, I have a poor experience with python c
>  binding.
>
>  Thanks
>
>  Francesco
>  _______________________________________________
>  system-config-printer-devel mailing list
>  system-config-printer-devel(a)lists.fedorahosted.org
>
>
> https://lists.fedorahosted.org/admin/lists/system-config-printer-devel@li...
>  


-- 
-----------------------------------------------------------
Francesco Faraone
GESTORES 1880 SAU

CTO

Pl. Urquinaona, 6, 16a planta
<http://maps.google.com/maps?q=Pla%C3%A7a+Urquinaona,+6,+Barcelona,+Spain&...
08010 Barcelona
Tel. [phone]
Fax. [phone]

ffaraone(a)gaa9.com




ADVERTIMENT LEGAL:
Les vostres dades es troben en un fitxer de Gestores 1880 S.A.U. Podeu
exercir els vostres drets d'accés, rectificació i cancel·lació  si ens
adreceu una comunicació per escrit mitjançant correo o correo electrònic
signat digitalment a l'adreça electrònica:

Gestores 1880 S.A.U.
plaça d'Urquinaona, 6, 16a planta,
08010 Barcelona
info(a)gaa9.com.

(Llei Orgànica 15/1999, de 13 de desembre, de Protecció de Dades de
Caràcter Personal).

El contingut d'aquest correu electrònic i els annexos adjunts són
estrictament confidencials. En cas que no sigueu el destinatari i hagiu
rebut aquest missatge per error, us agrairíem que ho comuniqueu
immediatament al remitent, sense difondre, emmagatzemar o copiar el seu
contingut.

ADVERTIMENT:
Noteu que no podem garantir que aquest missatge o qualsevol adjunt està
lliure de virus o que no ha estat interceptat i modificat.

ADVERTENCIA LEGAL:
Sus datos se encuentran recogidos en un fichero de Gestores 1880 S.A.U.
Puede ejercer su derecho de acceso, rectificación y cancelación si hace
llegar una comunicación por escrito mediante correo o correo electrónico
firmado digitalmente a la siguiente dirección:

Gestores 1880 S.A.U.
plaça d'Urquinaona, 6, 16a planta,
08010 Barcelona
info(a)gaa9.com <info(a)gaa9.com.com&gt;.

(Ley Orgánica 15/1999, de 13 de diciembre, de Protección de Datos de
Carácter Personal).

El contenido de este correo electrónico y sus anexos son estrictamente
confidenciales. En caso que no sea el destinatario y ha recibido este
mensaje por error, le agradeceríamos que lo comunique inmediatamente al
remitente, sin difundir, almacenar o copiar su contenido.

ADVERTENCIA:
Por favor, observe que no podemos garantizarle que este mensaje y sus
adjuntos sean libres de virus o que no hayan sido interceptados y
modificados.

DISCLAIMER:
Your personal data are stored in a file at Gestores 1880 S.A.U. If you want
to see, modify or delete your personal data, please send a mail or a
digitally signed electronic mail to the following address:

Gestores 1880 S.A.U.
plaça d'Urquinaona, 6, 16a planta,
08010 Barcelona
info(a)gaa9.com.

(Ley Orgánica 15/1999, de 13 de diciembre, de Protección de Datos de
Carácter Personal).

This message and any attachments contain privileged and confidential
information intended for the use of the addressee named above. If you are
not the intended recipient of this message, you are hereby notified that
any use, dissemination, distribution or reproduction of this message is
prohibited.

WARNING:
Please note that we cannot guarantee that this message or any attachment is
virus free or that it has not been intercepted and amended.