Fail if chroma subsampling selected when writing RGB JPEG

The user presumably doesn't intend to subsample the green and blue channels.
python-pillow · Dec 4, 2023 · c2fe53c · c2fe53c
1 parent c2b5686
commit c2fe53c
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 3 deletions.
diff --git a/Tests/test_file_jpeg.py b/Tests/test_file_jpeg.py
@@ -447,9 +447,15 @@ def getsampling(im):
             im = self.roundtrip(hopper(), subsampling=subsampling)
             assert getsampling(im) == (2, 2, 1, 1, 1, 1)
 
-        # RGB colorspace, no subsampling by default
-        im = self.roundtrip(hopper(), keep_rgb=True)
-        assert getsampling(im) == (1, 1, 1, 1, 1, 1)
+        # RGB colorspace
+        for subsampling in (-1, 0, "4:4:4"):
+            # "4:4:4" doesn't really make sense for RGB, but the conversion
+            # to an integer happens at a higher level
+            im = self.roundtrip(hopper(), keep_rgb=True, subsampling=subsampling)
+            assert getsampling(im) == (1, 1, 1, 1, 1, 1)
+        for subsampling in (1, "4:2:2", 2, "4:2:0", 3):
+            with pytest.raises(OSError):
+                self.roundtrip(hopper(), keep_rgb=True, subsampling=subsampling)
 
         with pytest.raises(TypeError):
             self.roundtrip(hopper(), subsampling="1:1:1")

diff --git a/docs/handbook/image-file-formats.rst b/docs/handbook/image-file-formats.rst
@@ -491,6 +491,9 @@ The :py:meth:`~PIL.Image.Image.save` method supports the following options:
     If this option is present and true, those images will be stored as RGB
     instead.
 
+    When this option is enabled, attempting to chroma-subsample RGB images
+    with the ``subsampling`` option will result in an error.
+
     .. versionadded:: 10.2.0
 
 **subsampling**

diff --git a/src/libImaging/JpegEncode.c b/src/libImaging/JpegEncode.c
@@ -144,6 +144,16 @@ ImagingJpegEncode(Imaging im, ImagingCodecState state, UINT8 *buf, int bytes) {
 #ifdef JCS_EXTENSIONS
                     case JCS_EXT_RGBX:
 #endif
+                        switch (context->subsampling) {
+                            case -1:  /* Default */
+                            case 0:   /* No subsampling */
+                                break;
+                            default:
+                                /* Would subsample the GB channels, which
+                                   doesn't make sense */
+                                state->errcode = IMAGING_CODEC_CONFIG;
+                                return -1;
+                        }
                         jpeg_set_colorspace(&context->cinfo, JCS_RGB);
                         break;
                     default: