If bitmap buffer is empty, do not render anything #8324
Conversation
| @@ -17,8 +15,7 @@ def _fuzz_font(self, font: ImageFont.FreeTypeFont) -> None: | |||
| draw.multiline_textbbox((10, 10), "ABC\nAaaa", font, stroke_width=2) | |||
| draw.text((10, 10), "Test Text", font=font, fill="#000") | |||
|
|
|||
| @skip_unless_feature("freetype2") | |||
| @skip_unless_feature_version("freetype2", "2.12.0") | |||
There was a problem hiding this comment.
Why change the version here?
There was a problem hiding this comment.
The test intended to catch OSError: Bitmap missing for glyph from
Lines 1028 to 1032 in b6f90c4
For FreeType2 versions before 2.12.0, FT_New_Face is returning an Unknown_File_Format error. It does that even on main at the moment, it's just not obvious because both are OSError. If I increase the specificity of the test, you can see this.
FreeType2 2.12.0 was released in March 2022, before #6846, so I expect this has been the case the whole time. Now that I've fixed the OSError: Bitmap missing for glyph error, the other error becomes obvious.
This was a font generated by OSS-Fuzz, so its purpose is not to be valid, but merely to trigger a security problem. We could modify the file to be valid, but in principle it would seem to be better to not modify test scenarios over time.
I suppose I could modify the test to run on FreeType2 < 2.12.0 as well, and just catch the error for that scenario if you want.
There was a problem hiding this comment.
Thank you for the explanation.
|
We might want to add a test that triggers the |
|
You're concerned about passing a glyph where bitmap buffer is empty to this block? |
|
Note then the glyph outline in question is not empty but degenerate. It contains two points but still produces a blank space. In this particular case, it results in a 0×1 bitmap, aka one empty row. Do not check for |
|
The glyph outline may not be empty, but the bitmap buffer is. From my understanding, we're safe to skip drawing and just advance if that is the case. I presume the size of the glyph does not affect the advance. |
|
Correct, just advance if the buffer is NULL regardless of its dimensions. You have sufficient error checking from FreeType calls. |
Resolves #8272
#6846 added an error if the bitmap buffer is empty.
Pillow/src/_imagingft.c
Lines 1028 to 1032 in d6cfebd
However, this new issue found the buffer is empty for a space character in a particularly font, which seems like a realistic scenario, and has been confirmed as valid by one of the FreeType maintainers - #8272 (comment)
So if the bitmap buffer is empty, this PR just doesn't draw anything, uses the
advances to update the position for the next character, and moves on.