ImageGrab.grabclipboard() keeps unnecessary temporary files around

[stephen-huan]

What did you do?

Call PIL.ImageGrab.grabclipboard() repeatedly.

What did you expect to happen?

A constant number of temp files to be created.

What actually happened?

The number of files in /tmp keeps increasing.

If there not an actual image in the clipboard, e.g. only text, then

xclip -selection clipboard -target image/png -out

will error with the message

Error: target image/png not available

while this error is caught on the latest Pillow (lines 146--148), the newly created tempfile is never removed.

Pillow/src/PIL/ImageGrab.py

Lines 146 to 151 in ab3d0c0

	if err:
	msg = f"{args[0]} error: {err.strip().decode()}"
	raise ChildProcessError(msg)
	im = Image.open(filepath)
	im.load()
	os.unlink(filepath)

This can cause tempfiles to accumulate with repeated calls of grabclipboard().

I think os.unlink(filepath) should be called before ChildProcessError is raised to remove the now unnecessary file, i.e.

        if err:
            os.unlink(filepath)
            msg = f"{args[0]} error: {err.strip().decode()}"
            raise ChildProcessError(msg)
        im = Image.open(filepath)
        im.load()
        os.unlink(filepath)

What are your OS, Python and Pillow versions?

• OS: archlinux
• Python: 3.11.3
• Pillow: 9.5.0 (or commit ab3d0c0)

[stephen-huan]

with a bit of additional testing I think the suggested

        if err:
            os.unlink(filepath)
            msg = f"{args[0]} error: {err.strip().decode()}"
            raise ChildProcessError(msg)
        im = Image.open(filepath)
        im.load()
        os.unlink(filepath)

should actually be something like

        if err:
            os.unlink(filepath)
            msg = f"{args[0]} error: {err.strip().decode()}"
            raise ChildProcessError(msg)
        im = None
        try:
            im = Image.open(filepath)
            im.load()
        except UnidentifiedImageError:
            pass
        os.unlink(filepath)

as after the first paste xclip will no longer error with the message

Error: target image/png not available

and simply write whatever text is in the clipboard to filepath,
which causes an UnidentifiedImageError.
This also makes it more consistent with the behavior for Windows and macOS.

[radarhere]

This also makes it more consistent with the behavior for Windows and macOS.

How so?

[stephen-huan]

In the sense that it should return None if there is not image data in the clipboard rather than erroring.

[radarhere]

I've created PR #7148 to remove the temporary file if a ChildProcessError occurs.

Regarding your second suggestion though, on macOS, it is returning None if the file is empty. On Windows, it is returning None if the image isn't PNG/DIB. What you're suggesting isn't either of those, and doesn't allow for any distinction between "this isn't image data" and "this is image data that Pillow can't successfully load". I don't think we should potentially silence Pillow failing to handle an image.

[stephen-huan]

Sure, I agree. I phrased the suggestion like "something like..." because I'm not sure of the proper way to do it. I just want to point out that even specifying -target image/png for xclip does not guarantee xclip does exactly one of
(a) raises a ChildProcessError or
(b) writes valid image data
since it sometimes just writes the contents of the clipboard without erroring, e.g. plaintext.

Looking at the code again, I'm not sure why a temporary file is even necessary in the first place.
And since the target is image/png, we can explicitly check whether the data is parsable as a PNG.

        p = subprocess.run(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
        if p.stderr:
            msg = f"{args[0]} error: {p.stderr.strip().decode()}"
            raise ChildProcessError(msg)
        data = p.stdout
        if isinstance(data, bytes):
            import io
            from PIL import PngImagePlugin

            data = io.BytesIO(data)
            try:
                return PngImagePlugin.PngImageFile(data)
            except SyntaxError:
                pass
        return None

There's still the problem of determining whether the file failed to load because it isn't PNG data at all, e.g. text, or whether the file failed to load because Pillow failed to load it. I'm not familiar with the Pillow API enough to know how to tell the difference, does PIL.PngImagePlugin.verify() work or something else?

[stephen-huan]

A bit orthogonal to this discussion, the latest stable documentation (9.5.0) for grabclipboard() is out of date.

It still says

Only macOS and Windows are currently supported.

when Linux support was added in 9.4.0

Something like
main...stephen-huan:Pillow:patch-1
should be fine but of course the claim

On Linux, an image, or None if the clipboard does not contain image data.

has not quite been substantiated yet.

[radarhere]

#7147 (comment)

If there not an actual image in the clipboard, e.g. only text, then

xclip -selection clipboard -target image/png -out
will error with the message

Error: target image/png not available

#7147 (comment)

I just want to point out that even specifying -target image/png for xclip does not guarantee xclip does exactly one of
(a) raises a ChildProcessError or
(b) writes valid image data
since it sometimes just writes the contents of the clipboard without erroring, e.g. plaintext.

In your first comment, you say that xclip doesn't print text data, and then the other comment you say that it does. Could you clarify? If you have found a way to get xclip to output text data from xclip -selection clipboard -target image/png -out, could you provide instructions? When I try, I get Error: target image/png not available.

[radarhere]

PR #7160 has been merged to update the grabclipboard() documentation to now state that Linux is supported. See https://pillow.readthedocs.io/en/latest/reference/ImageGrab.html#PIL.ImageGrab.grabclipboard

The stable documentation will be updated once the next version of Pillow is released.

[stephen-huan]

In your first comment, you say that xclip doesn't print text data, and then the other comment you say that it does. Could you clarify?

Sometimes it errors, sometimes it just prints text.

If you have found a way to get xclip to output text data from xclip -selection clipboard -target image/png -out, could you provide instructions? When I try, I get Error: target image/png not available.

Sure,

1. Open a new shell
2. Run xclip -selection clipboard -target image/png -out with text copied
3. xclip errors with Error: target image/png not available
4. Copy text
5. Run the same command again
6. xclip prints the copied text out without error

This is completely deterministic for me. If step 4 is not performed, i.e., if 2 is ran over and over again, it always errors with the message. But once the clipboard is updated by copying something, then it won't error again until a new shell is started.

[radarhere]

I'm unable to replicate.

xclip.mp4

[radarhere]

Looking at the code again, I'm not sure why a temporary file is even necessary in the first place.

I've created PR #7200 to no longer use a temporary file.

[stephen-huan]

I've created PR #7200 to no longer use a temporary file.

Thanks!

I'm unable to replicate.

It may be a quirk on my setup then, perhaps I should have specified that I was using the alacritty terminal emulator.

One difference might be the terminal emulator, in the clip you selected the text ubuntu@ubuntu, right-clicked to copy, and then when you went to run the command, the highlighted text stayed selected. You then right-clicked to copy again and ran the same command. Could it be that the PRIMARY buffer didn't change? For example, you could try clicking on different text and then copying it. Or you could try seeing if you can replicate on alacritty. When I select something, there is no contextual menu, I have to copy with <ctrl-c>. Then when I run go to run the command, the selection will automatically clear, forcing a re-selection if something else should be copied (see below).

xclip.mp4

[radarhere]

And since the target is image/png, we can explicitly check whether the data is parsable as a PNG.

If you've found a situation where xclip will just print out whatever it has, isn't it possible that the user copied a JPEG file, and that the data xclip is outputting on the second run is JPEG data that Pillow would currently understand? In case, returning None just because it isn't PNG data would seem unhelpful.

I don't think there's any conceptual way to distinguish between "this isn't image data" and "this is image data that Pillow can't successfully load".

[github-actions]

Closing this issue as no feedback has been received.