ICO image format: fix regression in writing when size >= 256

In commit c6c9304, the earlier size limit of 128 was raised to the
format's defined maximum of 256. But the required special storage of
this size in the image structures was not implemented. Hence,
attempting to store such big icons would result in invalid image
files.

Fix the size storing details, and add some autotests of ico format
writing since that was practically uncovered.

Task-number: QTBUG-53259
Change-Id: I00e17a04e90c32dcf1124ba5adaf53728fb74dc7
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>

src/plugins/imageformats/ico/qicohandler.cpp

@@ -54,7 +54,7 @@ QT_BEGIN_NAMESPACE
 typedef struct
 {
     quint8  bWidth;               // Width of the image
-    quint8  bHeight;              // Height of the image (times 2)
+    quint8  bHeight;              // Height of the image (actual height, not times 2)
     quint8  bColorCount;          // Number of colors in image (0 if >=8bpp) [ not ture ]
     quint8  bReserved;            // Reserved
     quint16 wPlanes;              // Color Planes
@@ -681,8 +681,8 @@ bool ICOReader::write(QIODevice *device, const QVector<QImage> &images)
             entries[i].bColorCount = 0;
             entries[i].bReserved = 0;
             entries[i].wBitCount = nbits;
-            entries[i].bHeight = image.height();
-            entries[i].bWidth = image.width();
+            entries[i].bHeight = image.height() < 256 ? image.height() : 0;  // 0 means 256
+            entries[i].bWidth = image.width() < 256 ? image.width() : 0;     // 0 means 256
             entries[i].dwBytesInRes = BMP_INFOHDR_SIZE + (bpl_bmp * image.height())
                 + (maskImage.bytesPerLine() * maskImage.height());
             entries[i].wPlanes = 1;
@@ -696,11 +696,11 @@ bool ICOReader::write(QIODevice *device, const QVector<QImage> &images)
             bmpHeaders[i].biClrImportant = 0;
             bmpHeaders[i].biClrUsed = entries[i].bColorCount;
             bmpHeaders[i].biCompression = 0;
-            bmpHeaders[i].biHeight = entries[i].bHeight * 2; // 2 is for the mask
+            bmpHeaders[i].biHeight = entries[i].bHeight ? entries[i].bHeight * 2 : 256 * 2; // 2 is for the mask
             bmpHeaders[i].biPlanes = entries[i].wPlanes;
             bmpHeaders[i].biSize = BMP_INFOHDR_SIZE;
             bmpHeaders[i].biSizeImage = entries[i].dwBytesInRes - BMP_INFOHDR_SIZE;
-            bmpHeaders[i].biWidth = entries[i].bWidth;
+            bmpHeaders[i].biWidth = entries[i].bWidth ? entries[i].bWidth : 256;
             bmpHeaders[i].biXPelsPerMeter = 0;
             bmpHeaders[i].biYPelsPerMeter = 0;
 

tests/auto/gui/image/qicoimageformat/tst_qicoimageformat.cpp

@@ -64,6 +64,8 @@ private slots:
     void nextImageDelay();
     void pngCompression_data();
     void pngCompression();
+    void write_data();
+    void write();
 
 private:
     QString m_IconPath;
@@ -335,6 +337,51 @@ void tst_QIcoImageFormat::pngCompression()
     QCOMPARE(image.height(), height);
 }
 
+void tst_QIcoImageFormat::write_data()
+{
+    QTest::addColumn<QSize>("inSize");
+    QTest::addColumn<QSize>("outSize");
+
+    QTest::newRow("64x64") << QSize(64, 64) << QSize(64, 64);
+    QTest::newRow("128x200") << QSize(128, 200) << QSize(128, 200);
+    QTest::newRow("256x256") << QSize(256, 256) << QSize(256, 256);
+    QTest::newRow("400x400") << QSize(400, 400) << QSize(256, 256);
+}
+
+void tst_QIcoImageFormat::write()
+{
+    QFETCH(QSize, inSize);
+    QFETCH(QSize, outSize);
+
+    QImage inImg;
+    {
+        QImageReader reader(m_IconPath + "/valid/Qt.ico");
+        reader.jumpToImage(4);
+        reader.setScaledSize(inSize);
+        inImg = reader.read();
+        QVERIFY(!inImg.isNull());
+        QCOMPARE(inImg.size(), inSize);
+    }
+
+    QBuffer buf;
+    {
+        buf.open(QIODevice::WriteOnly);
+        QImageWriter writer(&buf, "ico");
+        QVERIFY(writer.write(inImg));
+        buf.close();
+    }
+    {
+        buf.open(QIODevice::ReadOnly);
+        QImageReader reader(&buf);
+        QVERIFY(reader.canRead());
+        QCOMPARE(reader.format(), QByteArray("ico"));
+        QImage outImg = reader.read();
+        QVERIFY(!outImg.isNull());
+        QCOMPARE(outImg.size(), outSize);
+        buf.close();
+    }
+}
+
 QTEST_MAIN(tst_QIcoImageFormat)
 #include "tst_qicoimageformat.moc"
 

tests/auto/gui/image/qimagewriter/tst_qimagewriter.cpp

@@ -199,6 +199,7 @@ void tst_QImageWriter::writeImage_data()
     QTest::newRow("PBM: ship63") << QString("ship63.pbm") << true << QByteArray("pbm");
     QTest::newRow("XBM: gnus") << QString("gnus.xbm") << false << QByteArray("xbm");
     QTest::newRow("JPEG: beavis") << QString("beavis.jpg") << true << QByteArray("jpeg");
+    QTest::newRow("ICO: App") << QString("App.ico") << true << QByteArray("ico");
 }
 
 void tst_QImageWriter::writeImage()
@@ -260,7 +261,7 @@ void tst_QImageWriter::writeImage2_data()
     QTest::addColumn<QImage>("image");
 
     const QStringList formats = QStringList() << "bmp" << "xpm" << "png"
-                                              << "ppm"; //<< "jpeg";
+                                              << "ppm" << "ico"; //<< "jpeg";
     QImage image0(70, 70, QImage::Format_ARGB32);
     image0.fill(QColor(Qt::red).rgb());