From: Nigel Tao <nigeltao@golang.org>
Date: Thu, 27 Apr 2017 01:38:10 +0000 (+1000)
Subject: image/jpeg: fix extended sequential Huffman table selector (Th).
X-Git-Tag: go1.9beta1~440
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=35cbc3b55b9d5d159a5a7a160ea680b32f043bf8;p=gostls13.git

image/jpeg: fix extended sequential Huffman table selector (Th).

Previously, the package did not distinguish between baseline and
extended sequential images. Both are non-progressive images, but the Th
range differs between the two, as per Annex B of
https://www.w3.org/Graphics/JPEG/itu-t81.pdf

Extended sequential images are often emitted by the Guetzli encoder.

Fixes #19913

Change-Id: I3d0f9e16d5d374ee1c65e3a8fb87519de61cff94
Reviewed-on: https://go-review.googlesource.com/41831
Reviewed-by: David Symonds <dsymonds@golang.org>
---

diff --git a/src/image/jpeg/huffman.go b/src/image/jpeg/huffman.go
index 4f8fe8eff3..95aaf71e2f 100644
--- a/src/image/jpeg/huffman.go
+++ b/src/image/jpeg/huffman.go
@@ -101,7 +101,8 @@ func (d *decoder) processDHT(n int) error {
 			return FormatError("bad Tc value")
 		}
 		th := d.tmp[0] & 0x0f
-		if th > maxTh || !d.progressive && th > 1 {
+		// The baseline th <= 1 restriction is specified in table B.5.
+		if th > maxTh || (d.baseline && th > 1) {
 			return FormatError("bad Th value")
 		}
 		h := &d.huff[tc][th]
diff --git a/src/image/jpeg/reader.go b/src/image/jpeg/reader.go
index c5834219a3..a915e96a4c 100644
--- a/src/image/jpeg/reader.go
+++ b/src/image/jpeg/reader.go
@@ -48,7 +48,7 @@ const (
 )
 
 const (
-	sof0Marker = 0xc0 // Start Of Frame (Baseline).
+	sof0Marker = 0xc0 // Start Of Frame (Baseline Sequential).
 	sof1Marker = 0xc1 // Start Of Frame (Extended Sequential).
 	sof2Marker = 0xc2 // Start Of Frame (Progressive).
 	dhtMarker  = 0xc4 // Define Huffman Table.
@@ -126,9 +126,17 @@ type decoder struct {
 	blackPix    []byte
 	blackStride int
 
-	ri                  int // Restart Interval.
-	nComp               int
-	progressive         bool
+	ri    int // Restart Interval.
+	nComp int
+
+	// As per section 4.5, there are four modes of operation (selected by the
+	// SOF? markers): sequential DCT, progressive DCT, lossless and
+	// hierarchical, although this implementation does not support the latter
+	// two non-DCT modes. Sequential DCT is further split into baseline and
+	// extended, as per section 4.11.
+	baseline    bool
+	progressive bool
+
 	jfif                bool
 	adobeTransformValid bool
 	adobeTransform      uint8
@@ -596,6 +604,7 @@ func (d *decoder) decode(r io.Reader, configOnly bool) (image.Image, error) {
 
 		switch marker {
 		case sof0Marker, sof1Marker, sof2Marker:
+			d.baseline = marker == sof0Marker
 			d.progressive = marker == sof2Marker
 			err = d.processSOF(n)
 			if configOnly && d.jfif {
diff --git a/src/image/jpeg/scan.go b/src/image/jpeg/scan.go
index e1104d27c2..712e7e35ff 100644
--- a/src/image/jpeg/scan.go
+++ b/src/image/jpeg/scan.go
@@ -92,12 +92,13 @@ func (d *decoder) processSOS(n int) error {
 		}
 		totalHV += d.comp[compIndex].h * d.comp[compIndex].v
 
+		// The baseline t <= 1 restriction is specified in table B.3.
 		scan[i].td = d.tmp[2+2*i] >> 4
-		if scan[i].td > maxTh {
+		if t := scan[i].td; t > maxTh || (d.baseline && t > 1) {
 			return FormatError("bad Td value")
 		}
 		scan[i].ta = d.tmp[2+2*i] & 0x0f
-		if scan[i].ta > maxTh {
+		if t := scan[i].ta; t > maxTh || (d.baseline && t > 1) {
 			return FormatError("bad Ta value")
 		}
 	}
@@ -122,7 +123,8 @@ func (d *decoder) processSOS(n int) error {
 	// by the second-least significant bit, followed by the least
 	// significant bit.
 	//
-	// For baseline JPEGs, these parameters are hard-coded to 0/63/0/0.
+	// For sequential JPEGs, these parameters are hard-coded to 0/63/0/0, as
+	// per table B.3.
 	zigStart, zigEnd, ah, al := int32(0), int32(blockSize-1), uint32(0), uint32(0)
 	if d.progressive {
 		zigStart = int32(d.tmp[1+2*nComp])
@@ -177,7 +179,7 @@ func (d *decoder) processSOS(n int) error {
 					// The blocks are traversed one MCU at a time. For 4:2:0 chroma
 					// subsampling, there are four Y 8x8 blocks in every 16x16 MCU.
 					//
-					// For a baseline 32x16 pixel image, the Y blocks visiting order is:
+					// For a sequential 32x16 pixel image, the Y blocks visiting order is:
 					//	0 1 4 5
 					//	2 3 6 7
 					//
diff --git a/src/image/jpeg/writer.go b/src/image/jpeg/writer.go
index ce7728b484..a600499004 100644
--- a/src/image/jpeg/writer.go
+++ b/src/image/jpeg/writer.go
@@ -311,7 +311,7 @@ func (e *encoder) writeDQT() {
 	}
 }
 
-// writeSOF0 writes the Start Of Frame (Baseline) marker.
+// writeSOF0 writes the Start Of Frame (Baseline Sequential) marker.
 func (e *encoder) writeSOF0(size image.Point, nComponent int) {
 	markerlen := 8 + 3*nComponent
 	e.writeMarkerHeader(sof0Marker, markerlen)