support BITMAPV5HEADER in getimagesize() #249

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
@AsamK

AsamK commented Dec 30, 2012

the php function getimagesize() doesn't support Bitmaps with ICC color profiles (BITMAPV5HEADER).
The size information is at the same position as Bitmaps with a BITMAPV4HEADER, so just adding a check for the different header size should be enough
https://en.wikipedia.org/wiki/BMP_file_format#DIB_header_.28bitmap_information_header.29

@lstrojny

This comment has been minimized.

Show comment Hide comment
@lstrojny

lstrojny Jan 1, 2013

Contributor

Thanks for your PR. Please add a Testcase for the addition.

Contributor

lstrojny commented Jan 1, 2013

Thanks for your PR. Please add a Testcase for the addition.

@weltling

This comment has been minimized.

Show comment Hide comment
@weltling

weltling Jan 8, 2013

Contributor

@AsamK you should also fix ext\standard\tests\image\image_type_to_mime_type.phpt as it picks up your new image file. Then it should be fine i'd say.

Contributor

weltling commented Jan 8, 2013

@AsamK you should also fix ext\standard\tests\image\image_type_to_mime_type.phpt as it picks up your new image file. Then it should be fine i'd say.

@lstrojny

This comment has been minimized.

Show comment Hide comment
@lstrojny

lstrojny Jan 14, 2013

Contributor

@AsamK could you squash your commits, I’ll merge than.

Contributor

lstrojny commented Jan 14, 2013

@AsamK could you squash your commits, I’ll merge than.

support BITMAPV5HEADER in getimagesize()
https://en.wikipedia.org/wiki/BMP_file_format#DIB_header_.28bitmap_information_header.29

added test image:
ext/standard/tests/image/test1bpix.bmp

and adapted tests:
ext/standard/tests/image/getimagesize.phpt
ext/standard/tests/image/image_type_to_mime_type
@AsamK

This comment has been minimized.

Show comment Hide comment
@AsamK

AsamK Jan 14, 2013

I squashed them.
I hope I did it correctly, this is my first pull request on github.

AsamK commented Jan 14, 2013

I squashed them.
I hope I did it correctly, this is my first pull request on github.

@lstrojny

This comment has been minimized.

Show comment Hide comment
@lstrojny

lstrojny Jan 14, 2013

Contributor

Looks good! I’ll try to merge them now.

Contributor

lstrojny commented Jan 14, 2013

Looks good! I’ll try to merge them now.

@php-pulls

This comment has been minimized.

Show comment Hide comment
@php-pulls

php-pulls Jan 14, 2013

Comment on behalf of lstrojny at php.net:

Merged into 5.4, 5.5 and master. Thanks!

Comment on behalf of lstrojny at php.net:

Merged into 5.4, 5.5 and master. Thanks!

@php-pulls php-pulls closed this Jan 14, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment