bug#33684: DocView bombs out upon password protected PDFs

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

bug#33684: DocView bombs out upon password protected PDFs

積丹尼 Dan Jacobson
When trying to view a password protected PDF:
DocView: process pdf/ps->png changed status to exited abnormally with code 1.
Maybe it should ask for the password.
emacs-version "25.2.2"



Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Federico Tedin
積丹尼 Dan Jacobson <[hidden email]> writes:

> When trying to view a password protected PDF:
> DocView: process pdf/ps->png changed status to exited abnormally with code 1.
> Maybe it should ask for the password.
> emacs-version "25.2.2"

Would something like this make sense? A short patch that adds
`doc-view-pdf-password-protected-p', and an extra option to pass to
Ghostscript (-sPDFPassword=) when the command is invoked (only when the
PDF file is password protected). I tried it out by creating a
password-protected PDF using LibreOffice Writer, and then opening it
with find-file.


pdf-passwd.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Eli Zaretskii
> From: Federico Tedin <[hidden email]>
> Date: Sat, 26 Jan 2019 23:14:49 -0300

Thanks.

Tassilo, any comments on the proposal below?  Should I push it?

> 積丹尼 Dan Jacobson <[hidden email]> writes:
>
> > When trying to view a password protected PDF:
> > DocView: process pdf/ps->png changed status to exited abnormally with code 1.
> > Maybe it should ask for the password.
> > emacs-version "25.2.2"
>
> Would something like this make sense? A short patch that adds
> `doc-view-pdf-password-protected-p', and an extra option to pass to
> Ghostscript (-sPDFPassword=) when the command is invoked (only when the
> PDF file is password protected). I tried it out by creating a
> password-protected PDF using LibreOffice Writer, and then opening it
> with find-file.
>
>
> diff --git a/lisp/doc-view.el b/lisp/doc-view.el
> index df8a9fc70f..8691d2a3a6 100644
> --- a/lisp/doc-view.el
> +++ b/lisp/doc-view.el
> @@ -183,11 +183,16 @@ doc-view-pdf->png-converter-function
>  (defcustom doc-view-ghostscript-options
>    '("-dSAFER" ;; Avoid security problems when rendering files from untrusted
>      ;; sources.
> -    "-dNOPAUSE" "-sDEVICE=png16m" "-dTextAlphaBits=4"
> -    "-dBATCH" "-dGraphicsAlphaBits=4" "-dQUIET")
> +    "-dNOPAUSE" "-dTextAlphaBits=4" "-dBATCH"
> +    "-dGraphicsAlphaBits=4" "-dQUIET")
>    "A list of options to give to ghostscript."
>    :type '(repeat string))
>  
> +(defcustom doc-view-ghostscript-device "png16m"
> +  "Output device to give to ghostscript."
> +  :type 'string
> +  :version "27.1")
> +
>  (defcustom doc-view-resolution 100
>    "Dots per inch resolution used to render the documents.
>  Higher values result in larger images."
> @@ -950,16 +955,30 @@ doc-view-dvi->pdf
>      (list "-o" pdf dvi)
>      callback)))
>  
> +(defun doc-view-pdf-password-protected-p (pdf)
> +  "Using Ghostscript, check if a PDF file is password-protected."
> +  (with-temp-buffer
> +    (apply #'call-process doc-view-ghostscript-program nil (current-buffer)
> +           nil `(,@doc-view-ghostscript-options
> +                 "-sNODISPLAY"
> +                 ,pdf))
> +    (goto-char (point-min))
> +    (search-forward "This file requires a password for access." nil t)))
> +
>  (defun doc-view-pdf->png-converter-ghostscript (pdf png page callback)
> -  (doc-view-start-process
> -   "pdf/ps->png" doc-view-ghostscript-program
> -   `(,@doc-view-ghostscript-options
> -     ,(format "-r%d" (round doc-view-resolution))
> -     ,@(if page `(,(format "-dFirstPage=%d" page)))
> -     ,@(if page `(,(format "-dLastPage=%d" page)))
> -     ,(concat "-sOutputFile=" png)
> -     ,pdf)
> -   callback))
> +  (let ((pdf-passwd (if (doc-view-pdf-password-protected-p pdf)
> +                        (read-passwd "Enter password for PDF file: "))))
> +    (doc-view-start-process
> +     "pdf/ps->png" doc-view-ghostscript-program
> +     `(,@doc-view-ghostscript-options
> +       ,(concat "-sDEVICE=" doc-view-ghostscript-device)
> +       ,(format "-r%d" (round doc-view-resolution))
> +       ,@(if page `(,(format "-dFirstPage=%d" page)))
> +       ,@(if page `(,(format "-dLastPage=%d" page)))
> +       ,@(if pdf-passwd `(,(format "-sPDFPassword=%s" pdf-passwd)))
> +       ,(concat "-sOutputFile=" png)
> +       ,pdf)
> +     callback)))
>  
>  (defalias 'doc-view-ps->png-converter-ghostscript
>    'doc-view-pdf->png-converter-ghostscript)



Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Tassilo Horn-6
Eli Zaretskii <[hidden email]> writes:

Hi Federico, Dan & Eli,

>> From: Federico Tedin <[hidden email]>
>> Date: Sat, 26 Jan 2019 23:14:49 -0300
>
> Thanks.
>
> Tassilo, any comments on the proposal below? Should I push it?

It would be great if we could support opening password protected PDFs
also when we're using mupdf as a converter.  That has a "-p" option for
the very same sake which could be set analogously in
`doc-view-pdf->png-converter-mupdf'.

So, yes, please push it with that small addition.

Bye,
Tassilo

>> 積丹尼 Dan Jacobson <[hidden email]> writes:
>>
>> > When trying to view a password protected PDF:
>> > DocView: process pdf/ps->png changed status to exited abnormally with code 1.
>> > Maybe it should ask for the password.
>> > emacs-version "25.2.2"
>>
>> Would something like this make sense? A short patch that adds
>> `doc-view-pdf-password-protected-p', and an extra option to pass to
>> Ghostscript (-sPDFPassword=) when the command is invoked (only when the
>> PDF file is password protected). I tried it out by creating a
>> password-protected PDF using LibreOffice Writer, and then opening it
>> with find-file.
>>
>>
>> diff --git a/lisp/doc-view.el b/lisp/doc-view.el
>> index df8a9fc70f..8691d2a3a6 100644
>> --- a/lisp/doc-view.el
>> +++ b/lisp/doc-view.el
>> @@ -183,11 +183,16 @@ doc-view-pdf->png-converter-function
>>  (defcustom doc-view-ghostscript-options
>>    '("-dSAFER" ;; Avoid security problems when rendering files from untrusted
>>      ;; sources.
>> -    "-dNOPAUSE" "-sDEVICE=png16m" "-dTextAlphaBits=4"
>> -    "-dBATCH" "-dGraphicsAlphaBits=4" "-dQUIET")
>> +    "-dNOPAUSE" "-dTextAlphaBits=4" "-dBATCH"
>> +    "-dGraphicsAlphaBits=4" "-dQUIET")
>>    "A list of options to give to ghostscript."
>>    :type '(repeat string))
>>  
>> +(defcustom doc-view-ghostscript-device "png16m"
>> +  "Output device to give to ghostscript."
>> +  :type 'string
>> +  :version "27.1")
>> +
>>  (defcustom doc-view-resolution 100
>>    "Dots per inch resolution used to render the documents.
>>  Higher values result in larger images."
>> @@ -950,16 +955,30 @@ doc-view-dvi->pdf
>>      (list "-o" pdf dvi)
>>      callback)))
>>  
>> +(defun doc-view-pdf-password-protected-p (pdf)
>> +  "Using Ghostscript, check if a PDF file is password-protected."
>> +  (with-temp-buffer
>> +    (apply #'call-process doc-view-ghostscript-program nil (current-buffer)
>> +           nil `(,@doc-view-ghostscript-options
>> +                 "-sNODISPLAY"
>> +                 ,pdf))
>> +    (goto-char (point-min))
>> +    (search-forward "This file requires a password for access." nil t)))
>> +
>>  (defun doc-view-pdf->png-converter-ghostscript (pdf png page callback)
>> -  (doc-view-start-process
>> -   "pdf/ps->png" doc-view-ghostscript-program
>> -   `(,@doc-view-ghostscript-options
>> -     ,(format "-r%d" (round doc-view-resolution))
>> -     ,@(if page `(,(format "-dFirstPage=%d" page)))
>> -     ,@(if page `(,(format "-dLastPage=%d" page)))
>> -     ,(concat "-sOutputFile=" png)
>> -     ,pdf)
>> -   callback))
>> +  (let ((pdf-passwd (if (doc-view-pdf-password-protected-p pdf)
>> +                        (read-passwd "Enter password for PDF file: "))))
>> +    (doc-view-start-process
>> +     "pdf/ps->png" doc-view-ghostscript-program
>> +     `(,@doc-view-ghostscript-options
>> +       ,(concat "-sDEVICE=" doc-view-ghostscript-device)
>> +       ,(format "-r%d" (round doc-view-resolution))
>> +       ,@(if page `(,(format "-dFirstPage=%d" page)))
>> +       ,@(if page `(,(format "-dLastPage=%d" page)))
>> +       ,@(if pdf-passwd `(,(format "-sPDFPassword=%s" pdf-passwd)))
>> +       ,(concat "-sOutputFile=" png)
>> +       ,pdf)
>> +     callback)))
>>  
>>  (defalias 'doc-view-ps->png-converter-ghostscript
>>    'doc-view-pdf->png-converter-ghostscript)
>



Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Eli Zaretskii
> From: Tassilo Horn <[hidden email]>
> Cc: Federico Tedin <[hidden email]>,  [hidden email],  [hidden email]
> Date: Fri, 01 Feb 2019 11:12:51 +0100
>
> Eli Zaretskii <[hidden email]> writes:
>
> Hi Federico, Dan & Eli,
>
> >> From: Federico Tedin <[hidden email]>
> >> Date: Sat, 26 Jan 2019 23:14:49 -0300
> >
> > Thanks.
> >
> > Tassilo, any comments on the proposal below? Should I push it?
>
> It would be great if we could support opening password protected PDFs
> also when we're using mupdf as a converter.  That has a "-p" option for
> the very same sake which could be set analogously in
> `doc-view-pdf->png-converter-mupdf'.
>
> So, yes, please push it with that small addition.

Thanks.

Federico, would you please like to propose a patch with the addition
requested by Tassilo?



Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Federico Tedin
Sure! I’ll look into it.
Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Federico Tedin
In reply to this post by Eli Zaretskii
Here's the new version of the patch, which enables using MuPDF to open
password-protected PDF files.

One problem I encountered while writing it is that function
`doc-view-pdf->png-converter-mupdf' uses a small hack to add "draw" to
the arguments list passed to `doc-view-start-process', only when
`doc-view-pdfdraw-program' has the value "mutool". This is because the
"mudraw" command has been replaced at some point by the "mutool"
command, which requires passing "draw" as a subcommand to do the same
work. I ended up using the same hack in the new function I created, with
a reference to the original one, but I'm not sure this was the best
possible approach. Is there a cleaner way to solve this?

- Federico


pdf.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Tassilo Horn-6
Federico Tedin <[hidden email]> writes:

Hi Federico,

> Here's the new version of the patch, which enables using MuPDF to open
> password-protected PDF files.
>
> One problem I encountered while writing it is that function
> `doc-view-pdf->png-converter-mupdf' uses a small hack to add "draw" to
> the arguments list passed to `doc-view-start-process', only when
> `doc-view-pdfdraw-program' has the value "mutool". This is because the
> "mudraw" command has been replaced at some point by the "mutool"
> command, which requires passing "draw" as a subcommand to do the same
> work. I ended up using the same hack in the new function I created,
> with a reference to the original one, but I'm not sure this was the
> best possible approach. Is there a cleaner way to solve this?

You could have extracted that into its own function, e.g.,

(defun doc-view-pdfdraw-program-subcommand ()
  "Return the mutool subcommand replacing mudraw.

Recent mupdf distribution replaced mudraw with `mutool draw'."
  (when (string-match "mutool[^/\\]*$" doc-view-pdfdraw-program)
    '("draw")))

and use that at those two places.

Could you please commit the patch locally (including the ChangeLog style
commit message) and send it exported with "git format-patch"?

Bye,
Tassilo

PS: I had acually also accepted using the ghostscript password check
with mupdf, too.  But since you don't need ghostscript for PDFs if you
have mupdf, your approach is even better.




Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Federico Tedin
Hi Tassilo, thanks for your feedback. I've created a new patch with the
commit message, and also added an entry to NEWS. I'm attaching it here.

- Federico


pdf.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Tassilo Horn-6
Federico Tedin <[hidden email]> writes:

Hi Federico,

> Hi Tassilo, thanks for your feedback. I've created a new patch with
> the commit message, and also added an entry to NEWS. I'm attaching it
> here.

Looks perfect.  Small nitpick: we use * for bullet points instead of -
in the commit message, but I can amend that.  I can't push it from work,
so I'll do it this evening.

Thanks a lot for improving doc-view!
Tassilo



Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Eli Zaretskii
On February 4, 2019 7:44:43 AM GMT+02:00, Tassilo Horn <[hidden email]> wrote:

> Federico Tedin <[hidden email]> writes:
>
> Hi Federico,
>
> > Hi Tassilo, thanks for your feedback. I've created a new patch with
> > the commit message, and also added an entry to NEWS. I'm attaching
> it
> > here.
>
> Looks perfect.  Small nitpick: we use * for bullet points instead of -
> in the commit message, but I can amend that.  I can't push it from
> work,
> so I'll do it this evening.


When you do, please be sure to amend the log message to mention the bug number.

Also, the doc strings of 2 of the new functions need to be reformatted to have the first line be a complete sentence.  (Just move the second sentence to the next line, I'd say.)

Thanks.



Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Tassilo Horn-6
Eli Zaretskii <[hidden email]> writes:

>> Looks perfect.  Small nitpick: we use * for bullet points instead of -
>> in the commit message, but I can amend that.  I can't push it from
>> work, so I'll do it this evening.
>
> When you do, please be sure to amend the log message to mention the
> bug number.

Ah, right!

> Also, the doc strings of 2 of the new functions need to be reformatted
> to have the first line be a complete sentence.  (Just move the second
> sentence to the next line, I'd say.)

Will do.

Bye,
Tassilo



Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Tassilo Horn-6
In reply to this post by Eli Zaretskii
Eli Zaretskii <[hidden email]> writes:

>> > Hi Tassilo, thanks for your feedback. I've created a new patch with
>> > the commit message, and also added an entry to NEWS. I'm attaching
>> it
>> > here.
>>
>> Looks perfect.  Small nitpick: we use * for bullet points instead of
>> - in the commit message, but I can amend that.  I can't push it from
>> work, so I'll do it this evening.
>
> When you do, please be sure to amend the log message to mention the
> bug number.
>
> Also, the doc strings of 2 of the new functions need to be reformatted
> to have the first line be a complete sentence.  (Just move the second
> sentence to the next line, I'd say.)

Pushed with the aforementioned changes.

Thanks again, Federico!
Tassilo



Reply | Threaded
Open this post in threaded view
|

bug#33684: DocView bombs out upon password protected PDFs

Federico Tedin
> Pushed with the aforementioned changes.
>
> Thanks again, Federico!
> Tassilo

No problem! Thank you.