Bug 707307 - Removing `finddevice` causes problems
Summary: Removing `finddevice` causes problems
Status: RESOLVED WONTFIX
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: PS Interpreter (show other bugs)
Version: unspecified
Hardware: PC Linux
: P2 normal
Assignee: Default assignee
URL:
Keywords:
: 707309 (view as bug list)
Depends on:
Blocks:
 
Reported: 2023-11-03 08:46 UTC by Werner Lemberg
Modified: 2023-11-07 08:36 UTC (History)
1 user (show)

See Also:
Customer:
Word Size: ---


Attachments
utility file (3.91 KB, application/postscript)
2023-11-06 17:04 UTC, Ken Sharp
Details
updated utility file (4.59 KB, application/postscript)
2023-11-07 08:36 UTC, Ken Sharp
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Werner Lemberg 2023-11-03 08:46:54 UTC
In version 10.02.1 the `finddevice` command was removed.  This breaks LilyPond, which uses this command in the generated PS output that gets eventually converted to PDF with gs.

The standard procedure to remove a command is to announce in the version *before* that it gets removed in the *next* version.  This didn't happen, AFAIK.  Additionally, the removal isn't mentioned in the release notes, which shouldn't happen either.  And, to be honest, only X.XX releases should introduce incompatible changes, not X.XX.X bugfix releases.

BTW, where are the older release notes?  It seems they completely disappeared from the gs documentation site, or are very well hidden.  This exacerbates the whole issue.

As a first remedy for other users, please make it *immediately* visible in the 'Incompatible changes' section of the release notes (https://ghostscript.readthedocs.io/en/gs10.02.1/News.html?utm_source=ghostscript&utm_medium=website&utm_content=inline-link&_gl=1*1v0iaqk*_ga*MTU4OTUxNzExNy4xNjk4OTk0NTg3*_ga_JZTN4VTL9M*MTY5ODk5NDU4Ny4xLjEuMTY5ODk5OTk2OS42MC4wLjA) – it seems that this section is empty currently.

In the long run I ask you to be more careful with such changes.  People are right now updating ghostscript in all GNU/Linux distributions, which immediately makes LilyPond fail.  This is really, really bad.
Comment 1 Ken Sharp 2023-11-03 09:03:35 UTC
(In reply to Werner Lemberg from comment #0)
> In version 10.02.1 the `finddevice` command was removed.  This breaks
> LilyPond, which uses this command in the generated PS output that gets
> eventually converted to PDF with gs.

Why ? You shouldn't need to do that. If you do want to change device you can do so by using setpagedevice with the /OutputDevice key. This is the PostScript-standard way of changing the output device.


> The standard procedure to remove a command is to announce in the version
> *before* that it gets removed in the *next* version.  This didn't happen,
> AFAIK.

No, it didn't. This is a security release, we can't reasonably issue warnings in the previous release about changes that are going to happen in a security release.


> which shouldn't happen either.  And, to be honest, only X.XX releases should
> introduce incompatible changes, not X.XX.X bugfix releases.

It isn't a bugfix release, it's a security fix release.

 
> BTW, where are the older release notes?

As noted in News.html:

"From 9.55.0 onwards, in recognition of how unwieldy very large HTML files can become (History9.html had reached 8.1Mb!), we intend to only include the summary highlights (above).

For anyone wanting the full details of the changes in a release, we ask them to look at the history in our public git repository: ghostpdl-10.00.0 log.

If this change does not draw negative feedback, History?.htm file(s) will be removed from the release archives"


> In the long run I ask you to be more careful with such changes.  People are
> right now updating ghostscript in all GNU/Linux distributions, which
> immediately makes LilyPond fail.  This is really, really bad.

It is not possible for us to know all the packages using non-standard PostScript, so it isn't reasonable to expect us to 'be more careful'.

Frankly all such usage should cease. The various non-standard extensions have, in recent years, been increasingly demonstrated to be gaping security holes, which is why we have removed them. This is likely to continue to be the case.

We do not take such decisions lightly, but the correct solution is to stop using these non-standard extensions.
Comment 2 Werner Lemberg 2023-11-03 09:28:50 UTC
> > In version 10.02.1 the `finddevice` command was removed.  This breaks
> > LilyPond, which uses this command in the generated PS output that gets
> > eventually converted to PDF with gs.
> 
> Why ?

For historical reasons.

> You shouldn't need to do that. If you do want to change device you can
> do so by using setpagedevice with the /OutputDevice key. This is the
> PostScript-standard way of changing the output device.

Yes, thanks, we will change that, but this is not what I'm complaining here.

> > The standard procedure to remove a command is to announce in the version
> > *before* that it gets removed in the *next* version.  This didn't happen,
> > AFAIK.
> 
> No, it didn't. This is a security release, we can't reasonably issue
> warnings in the previous release about changes that are going to happen in a
> security release.

Well, *nothing* on the News page says that it is a security release, and I'm not allowed to access bug #707225 – how shall I know?

> > BTW, where are the older release notes?
> 
> As noted in News.html: [...]
> 
> If this change does not draw negative feedback, History?.htm file(s) will be
> removed from the release archives"

OK, herewith you get my negative feedback: I consider a removal of these files as bad.  Tedious searching through a git repository is not an adequate replacement for me.

> > In the long run I ask you to be more careful with such changes.  People are
> > right now updating ghostscript in all GNU/Linux distributions, which
> > immediately makes LilyPond fail.  This is really, really bad.
> 
> It is not possible for us to know all the packages using non-standard
> PostScript, so it isn't reasonable to expect us to 'be more careful'.  [...]
> 
> We do not take such decisions lightly, but the correct solution is to stop
> using these non-standard extensions.

You are completely missing the point.  `finddevice` was a *documented* function, removing it is a backward-incompatible change that deserves a prominent mentioning in the 'Incompatible changes' section.

I agree that non-standard extensions should be avoided, but again, this is a different discussion.
Comment 3 Ken Sharp 2023-11-03 11:33:16 UTC
(In reply to Werner Lemberg from comment #2)

> > No, it didn't. This is a security release, we can't reasonably issue
> > warnings in the previous release about changes that are going to happen in a
> > security release.
> 
> Well, *nothing* on the News page says that it is a security release, and I'm
> not allowed to access bug #707225 – how shall I know?

Well all the x.xx.x releases are security releases. The reason you can't access 707225 is precisely because it is a security issue and therefore not public. In fact that isn't (IIRC) the actual relevant bug, there were 18 commits covering a number of different reports, only one has a CVE.

Chris acknowledges the point, however, and we'll be sure to make that note in future.


> > If this change does not draw negative feedback, History?.htm file(s) will be
> > removed from the release archives"
> 
> OK, herewith you get my negative feedback: I consider a removal of these
> files as bad.  Tedious searching through a git repository is not an adequate
> replacement for me.

OK your feedback is noted, its the first piece of feedback in 2 years though, so I can't promise any changes.


> `finddevice` was a *documented*
> function, removing it is a backward-incompatible change that deserves a
> prominent mentioning in the 'Incompatible changes' section.

As noted above, we will do that in future.
Comment 4 Chris Liddell (chrisl) 2023-11-03 11:57:42 UTC
(In reply to Ken Sharp from comment #3)
> (In reply to Werner Lemberg from comment #2)
> 
<SNIP> 
> 
> > > If this change does not draw negative feedback, History?.htm file(s) will be
> > > removed from the release archives"
> > 
> > OK, herewith you get my negative feedback: I consider a removal of these
> > files as bad.  Tedious searching through a git repository is not an adequate
> > replacement for me.
> 
> OK your feedback is noted, its the first piece of feedback in 2 years
> though, so I can't promise any changes.

I would also note that for 8+ years prior to that change in policy, the changelog was nothing but an html formatted version of a "git log <release 1 SHA>....<release 2 SHA>" so in practice, it is not much different. Which is why we decided on the change.

Not documenting the incompatible changes was an oversight, and I apologise for that.
Comment 5 Werner Lemberg 2023-11-04 15:33:36 UTC
Thanks.

BTW, what's the replacement for the sequence `findprotodevice copydevice`?  The LilyPond source code contains the following comment:

```
;; We use `findprotodevice` because `finddevice` always returns
;; the same device instance and we can't reset the page number of
;; the device. `findprotodevice copydevice` creates a new device
;; instance each time, which can reset the page number.
```

AFAICS, `setpagedevice` behaves identical to `finddevice`...
Comment 6 Werner Lemberg 2023-11-04 15:59:30 UTC
My last comment is too cryptic, sorry.  The pre-gs-10.02.1 code we use is

```
mark
  /OutputFile (page%d.png)
  ...
  (png16m) findprotodevice copydevice
putdeviceprops setdevice
```

so that multiple input PDFs are converted to PNG file by file, and the above calling sequence ensures that `%d` is reset for each input file.
Comment 7 Ken Sharp 2023-11-04 16:10:50 UTC
(In reply to Werner Lemberg from comment #6)

> mark
>   /OutputFile (page%d.png)
>   ...
>   (png16m) findprotodevice copydevice
> putdeviceprops setdevice
> ```
> 
> so that multiple input PDFs are converted to PNG file by file, and the above
> calling sequence ensures that `%d` is reset for each input file.

I believe something like:

save
<<
/OutputDevice /png16m
/OutputFile (page%d.png)
>> setpagedevice
....
do stuff here
....
restore

Provided you didn't start with the device being png16m that will switch to the png16m device, then destroy the device on restore. Running the sequence again will create a new device initialised to zero.

I think it may also possible to change the page number, since it is stored as a key in the page device dictionary, but I'd have to check that, memory is notoriously fallible about such details and old devices don't always follow the rules anyway.

I'll try and remember to look on Monday.

If you have further questions it would be better to pitch them in our Discord channel, or via freenode IRC which is bridged to our Discord channel.
Comment 8 Ken Sharp 2023-11-06 16:54:39 UTC
*** Bug 707309 has been marked as a duplicate of this bug. ***
Comment 9 Ken Sharp 2023-11-06 17:04:59 UTC
Created attachment 25006 [details]
utility file

So, as I said, I looked into this today.

There is no actual bug here, the code is working as intended. Whether that's desirable is a different question.

The device creation caches a copy of the device, keyed by name, when the device is first created. Restoring it away does not dispose of the cached copy. When we next come to set a device we start by looking to see if we have a cached copy and if we do, we use that.

So that defeats save/restore, which to me seems wrong. But we can't simply change that without warning, obviously, since this isn't a security issue.

Therefore I have added code to gs_init.ps which allows a user to flush a named device from the cache, or flush all devices from the cache except the current device.

Yes this is ugly, but its less ugly than 'findprotodevice', and for now at least I only intend it as a stop-gap. My intention is to deprecate and then remove the caching unless someone can come up with a good reason not to. From my reading of the PLRM this is plain wrong, when the device is restored away it should go, and selecting a device should not choose any old one that happens to be lying around (unless, obviously, the selection is for the current device).

I've attached the code here as well, along with a simple demonstration of its use. This file requires to be run with -dNOSAFER or --permit-file-write=d:/temp/* or the paths to be changed in the file (I'd recommend changing the paths no matter what).
Comment 10 Werner Lemberg 2023-11-06 17:31:42 UTC
Thanks a lot!  This is very helpful, and I will check it in detail soon.

I wonder what license your code has.  I looked around at bugs.ghostscript.com and couldn't find any hint whether (public) bug reports and attached files are public domain or something else...
Comment 11 Ken Sharp 2023-11-06 18:59:29 UTC
(In reply to Werner Lemberg from comment #10)

> I wonder what license your code has.

Ghostscript is licensed under the AGPL version 3. You should have a license file of some kind in the installation directory. Its possible packagers might alter the filename but they should include the license itself. On Windows it's called 'COPYING' and is in the doc subdirectory.

Also when you run Ghostscript the banner should say something like:

GPL Ghostscript 10.03.0 (2023-09-13)
Copyright (C) 2023 Artifex Software, Inc.  All rights reserved.
This software is supplied under the GNU AGPLv3 and comes with NO WARRANTY:
see the file COPYING for details.

It's also on our web site and probably somewhere in the documentation too:

https://ghostscript.com/licensing/index.html#open-source


>  I looked around at
> bugs.ghostscript.com and couldn't find any hint whether (public) bug reports
> and attached files are public domain or something else...

OK so that's a slightly different question. Bugzilla is basically open, at least for free users. You should consider any files you attach to be in the public domain. Obviously you can put your copyright in the files, etc. but that won't stop people grabbing it if they want to.

If that isn't acceptable we can make attachments private, but its best to contact us *before* posting a file (you can just open the report and stick a request for assistance with that in the comment).

That's simply because if we don't co-ordinate it there can be a period when the files are publicly available.

Files from commercial customers we generally don't attach at all, unless they are reduced to the point where they can't be considered sensitive.

Security bugs are limited to the security group. That includes the Artifex developers and a number of Linux package maintainers (there may be a few other people too, I'm a bit hazy on the exact membership), so that they are aware of important pending security issues.

If it helps you're welcome to use the code posted here as is. It should continue to work, even if we do remove the caching, most of the code is actually checking conditions to make sure that continues to be the case.
Comment 12 Werner Lemberg 2023-11-07 06:01:53 UTC
Thanks for the explanation; I'll add a comment to your code, mentioning that it is in the public domain.

Note that I had to change the line

```
        dup currentpagedevice /Name get eq
```

to

```
        dup currentpagedevice /OutputDevice get eq
```

to avoid an error; this is probably because of using `gsapi_run_string`.
Comment 13 Ken Sharp 2023-11-07 08:36:29 UTC
Created attachment 25007 [details]
updated utility file

(In reply to Werner Lemberg from comment #12)

> Note that I had to change the line
> 
> ```
>         dup currentpagedevice /Name get eq
> ```
> 
> to
> 
> ```
>         dup currentpagedevice /OutputDevice get eq
> ```
> 
> to avoid an error; this is probably because of using `gsapi_run_string`.

That's odd. I tested that and the device is definitely named in the /Name key. Using gsapi_run_string shouldn't make any difference, though I was working from the interactive prompt and a file on the command line.

It appears that the dictionary returned by currentpagedevice (for me anyway) contains both keys. I'll modify the code to check for both.

Attached updated file.