Fix: bitmaps with BITMAPV3INFOHEADER cannot be imported

[Gymnasiast]

These files are reportedly written by Photoshop and GIMP and based on a specification by Microsoft. More information can be found here:

• https://en.wikipedia.org/wiki/BMP_file_format#DIB_header_(bitmap_information_header)
• https://formats.kaitai.io/bmp/
• https://web.archive.org/web/20150127132443/https://forums.adobe.com/message/3272950

Here is a file using this header: TESTRED1.bmp.zip
Besides the uncommon header, it also writes bitfields, which aren’t necessary at 24 or 32 bits.

This file can be opened correctly by GIMP, Eye of Gnome and Chromium.

I used the following script to test:

<?php
$file = 'TESTRED1.bmp';

$gd = imagecreatefrombmp($file);
var_dump($gd);

Output without the fix:

PHP Warning:  imagecreatefrombmp(): "/home/michael/Bureaublad/TESTRED1.bmp" is not a valid BMP file in /home/michael/testgd.php on line 4
PHP Stack trace:
PHP   1. {main}() /home/michael/testgd.php:0
PHP   2. imagecreatefrombmp($filename = '/home/michael/Bureaublad/TESTRED1.bmp') /home/michael/testgd.php:4
/home/michael/testgd.php:5:
bool(false)

Output with the fix:

object(GdImage)#1 (0) {
}

Additionally, this file now also works correctly whereas it did not before. It contains a regular V5 header, but it also contains bitfields, which as I mentioned as unnecessary at 24 or 32 bits: TESTRED2.bmp.zip

This is my first time contributing to PHP, I hope I followed all steps correctly.

[Gymnasiast]

Hm, looking at https://github.com/php/php-src/blob/ede8adb2e2013eafe67edb64fc8d5fa62f40cb7d/CONTRIBUTING.md#pull-requests , it seems like I should have created this against the PHP-8.1 branch, is that correct? If so, I will try and do that in the coming days.

[Girgias]

Hm, looking at https://github.com/php/php-src/blob/ede8adb2e2013eafe67edb64fc8d5fa62f40cb7d/CONTRIBUTING.md#pull-requests , it seems like I should have created this against the PHP-8.1 branch, is that correct? If so, I will try and do that in the coming days.

For bug fixes yes, please target the lowest maintained branch (8.1) the merging up into 8.2 and master will be taken care off when we merge this.

Ideally, could you also add a regression test with a small image so that we don't break this again in the future.

Now, I am surprised that the failure without the fix is telling you the function is undefined, that looks wrong, are you sure you tested with a build that has support for this extension enabled?

[Gymnasiast]

Now, I am surprised that the failure without the fix is telling you the function is undefined, that looks wrong, are you sure you tested with a build that has support for this extension enabled?

Initially I built without GD by accident, but I quickly noticed that and fixed it. I just copied the wrong output to my post. I had a few too many windows open :')

I will retarget the PHP-8.1 branch and also re-run my tests again, and update the output in my first post accordingly. I will also try and see if I can add a test - the image I used in the first post is only 16×21 and perfect for the purpose.

[Gymnasiast]

I think that should address everything. I included both test files that are in the first post. As both of these should evaluate to the exact same image (as they only differ in terms of their header), I used a single image for the expected result of both. I ran the test suite locally and it does list the new ones:

PASS imagecreatefrombmp() - test image with BITMAPV3INFOHEADER and (unnecessary) bitfields [ext/gd/tests/imagecreatefrombmp-v3info-32bpp-bitfields-11027.phpt] 
PASS imagecreatefrombmp() - test image with BITMAPV5HEADER, 32bpp and (unnecessary) bitfields [ext/gd/tests/imagecreatefrombmp-v5-32bpp-bitfields-11027.phpt] 

[cmb69]

Sorry, I'm a "bit" late, but it seems to me that this is change which is not in https://github/libgd/libgd, so it should be suggested to them first; we can still port that when accepted upstream.

@devnexen, thoughts?

[devnexen]

agreed it should be upstreamed, the change looks fairly small so you may have your chance it get reviewed in a timely manner.

[Gymnasiast]

Thank you for your answers, I will try to get it upstreamed in libgd first.

[cmb69]

I will try to get it upstreamed in libgd first.

Thank you! For the time being, I'm switching this PR to draft status.