forked from minhngoc25a/freetype2
[base] Fix `FT_Open_Face`'s handling of user-supplied streams.
This was already true (though undocumented) most of the time, but not if `FT_NEW` inside `FT_Stream_New` failed or if the `FT_OPEN_XXX` flags were bad. Normally, `FT_Open_Face` calls `FT_Stream_New`, which returns the user-supplied stream unchanged, and in case of any subsequent error in `FT_Open_Face`, the stream is closed via `FT_Stream_Free`. Up to now, however, `FT_Stream_New` allocates a new stream even if it is already given one by the user. If this allocation fails, the user-supplied stream is not returned to `FT_Open_Face` and never closed. Moreover, the user cannot detect this situation: all they see is that `FT_Open_Face` returns `FT_Err_Out_Of_Memory`, but that can also happen after a different allocation fails within the main body of `FT_Open_Face`, when the user's stream has already been closed by `FT_Open_Face`. It is plausible that the user stream's `close` method frees memory allocated for the stream object itself, so the user cannot defensively free it upon `FT_Open_Face` failure lest it ends up doubly freed. All in all, this ends up leaking the memory/resources used by user's stream. Furthermore, `FT_Stream_New` simply returns an error if the `FT_OPEN_XXX` flags are unsupported, which can mean either an invalid combination of flags or a perfectly innocent `FT_OPEN_STREAM` on a FreeType build that lacks stream support. With this patch, the user-supplied stream is closed even in these cases, so the user can be sure that if `FT_Open_Face` failed, the stream is definitely closed. * src/base/ftobjs.c (FT_Stream_New): Don't allocate a buffer unnecessarily. Move error-handling code to make the control flow more obvious. Close user-supplied stream if the flags are unsupported. `FT_Stream_Open` always sets `pathname.pointer`, so remove the redundant (re)assignment. None of the `FT_Stream_Open...` functions uses `stream->memory`, so keep just one assignment at the end, shared among all possible control flow paths. ('Unsupported flags' that may need a stream closure can be either an invalid combination of multiple `FT_OPEN_XXX` mode flags or a clean `FT_OPEN_STREAM` flag on a FreeType build that lacks stream support.)
This commit is contained in:
parent
a4c8f21ae7
commit
5d27b10f4c
46
ChangeLog
46
ChangeLog
|
@ -1,3 +1,49 @@
|
||||||
|
2021-07-13 Oleg Oshmyan <chortos@inbox.lv>
|
||||||
|
|
||||||
|
[base] Fix `FT_Open_Face`'s handling of user-supplied streams.
|
||||||
|
|
||||||
|
This was already true (though undocumented) most of the time, but
|
||||||
|
not if `FT_NEW` inside `FT_Stream_New` failed or if the
|
||||||
|
`FT_OPEN_XXX` flags were bad.
|
||||||
|
|
||||||
|
Normally, `FT_Open_Face` calls `FT_Stream_New`, which returns the
|
||||||
|
user-supplied stream unchanged, and in case of any subsequent error
|
||||||
|
in `FT_Open_Face`, the stream is closed via `FT_Stream_Free`.
|
||||||
|
|
||||||
|
Up to now, however, `FT_Stream_New` allocates a new stream even if
|
||||||
|
it is already given one by the user. If this allocation fails, the
|
||||||
|
user-supplied stream is not returned to `FT_Open_Face` and never
|
||||||
|
closed. Moreover, the user cannot detect this situation: all they
|
||||||
|
see is that `FT_Open_Face` returns `FT_Err_Out_Of_Memory`, but that
|
||||||
|
can also happen after a different allocation fails within the main
|
||||||
|
body of `FT_Open_Face`, when the user's stream has already been
|
||||||
|
closed by `FT_Open_Face`. It is plausible that the user stream's
|
||||||
|
`close` method frees memory allocated for the stream object itself,
|
||||||
|
so the user cannot defensively free it upon `FT_Open_Face` failure
|
||||||
|
lest it ends up doubly freed. All in all, this ends up leaking the
|
||||||
|
memory/resources used by user's stream.
|
||||||
|
|
||||||
|
Furthermore, `FT_Stream_New` simply returns an error if the
|
||||||
|
`FT_OPEN_XXX` flags are unsupported, which can mean either an
|
||||||
|
invalid combination of flags or a perfectly innocent
|
||||||
|
`FT_OPEN_STREAM` on a FreeType build that lacks stream support.
|
||||||
|
With this patch, the user-supplied stream is closed even in these
|
||||||
|
cases, so the user can be sure that if `FT_Open_Face` failed, the
|
||||||
|
stream is definitely closed.
|
||||||
|
|
||||||
|
* src/base/ftobjs.c (FT_Stream_New): Don't allocate a buffer
|
||||||
|
unnecessarily.
|
||||||
|
Move error-handling code to make the control flow more obvious.
|
||||||
|
Close user-supplied stream if the flags are unsupported.
|
||||||
|
`FT_Stream_Open` always sets `pathname.pointer`, so remove the
|
||||||
|
redundant (re)assignment. None of the `FT_Stream_Open...` functions
|
||||||
|
uses `stream->memory`, so keep just one assignment at the end,
|
||||||
|
shared among all possible control flow paths.
|
||||||
|
('Unsupported flags' that may need a stream closure can be either an
|
||||||
|
invalid combination of multiple `FT_OPEN_XXX` mode flags or a clean
|
||||||
|
`FT_OPEN_STREAM` flag on a FreeType build that lacks stream
|
||||||
|
support.)
|
||||||
|
|
||||||
2021-07-13 Oleg Oshmyan <chortos@inbox.lv>
|
2021-07-13 Oleg Oshmyan <chortos@inbox.lv>
|
||||||
|
|
||||||
[base] Reject combinations of incompatible `FT_OPEN_XXX` flags.
|
[base] Reject combinations of incompatible `FT_OPEN_XXX` flags.
|
||||||
|
|
|
@ -2301,6 +2301,10 @@ FT_BEGIN_HEADER
|
||||||
* See the discussion of reference counters in the description of
|
* See the discussion of reference counters in the description of
|
||||||
* @FT_Reference_Face.
|
* @FT_Reference_Face.
|
||||||
*
|
*
|
||||||
|
* If `FT_OPEN_STREAM` is set in `args->flags`, the stream in
|
||||||
|
* `args->stream` is automatically closed before this function returns
|
||||||
|
* any error (including `FT_Err_Invalid_Argument`).
|
||||||
|
*
|
||||||
* @example:
|
* @example:
|
||||||
* To loop over all faces, use code similar to the following snippet
|
* To loop over all faces, use code similar to the following snippet
|
||||||
* (omitting the error handling).
|
* (omitting the error handling).
|
||||||
|
|
|
@ -212,14 +212,12 @@
|
||||||
mode = args->flags &
|
mode = args->flags &
|
||||||
( FT_OPEN_MEMORY | FT_OPEN_STREAM | FT_OPEN_PATHNAME );
|
( FT_OPEN_MEMORY | FT_OPEN_STREAM | FT_OPEN_PATHNAME );
|
||||||
|
|
||||||
if ( FT_NEW( stream ) )
|
|
||||||
goto Exit;
|
|
||||||
|
|
||||||
stream->memory = memory;
|
|
||||||
|
|
||||||
if ( mode == FT_OPEN_MEMORY )
|
if ( mode == FT_OPEN_MEMORY )
|
||||||
{
|
{
|
||||||
/* create a memory-based stream */
|
/* create a memory-based stream */
|
||||||
|
if ( FT_NEW( stream ) )
|
||||||
|
goto Exit;
|
||||||
|
|
||||||
FT_Stream_OpenMemory( stream,
|
FT_Stream_OpenMemory( stream,
|
||||||
(const FT_Byte*)args->memory_base,
|
(const FT_Byte*)args->memory_base,
|
||||||
(FT_ULong)args->memory_size );
|
(FT_ULong)args->memory_size );
|
||||||
|
@ -230,8 +228,12 @@
|
||||||
else if ( mode == FT_OPEN_PATHNAME )
|
else if ( mode == FT_OPEN_PATHNAME )
|
||||||
{
|
{
|
||||||
/* create a normal system stream */
|
/* create a normal system stream */
|
||||||
|
if ( FT_NEW( stream ) )
|
||||||
|
goto Exit;
|
||||||
|
|
||||||
error = FT_Stream_Open( stream, args->pathname );
|
error = FT_Stream_Open( stream, args->pathname );
|
||||||
stream->pathname.pointer = args->pathname;
|
if ( error )
|
||||||
|
FT_FREE( stream );
|
||||||
}
|
}
|
||||||
else if ( ( mode == FT_OPEN_STREAM ) && args->stream )
|
else if ( ( mode == FT_OPEN_STREAM ) && args->stream )
|
||||||
{
|
{
|
||||||
|
@ -239,21 +241,24 @@
|
||||||
|
|
||||||
/* in this case, we do not need to allocate a new stream object */
|
/* in this case, we do not need to allocate a new stream object */
|
||||||
/* since the caller is responsible for closing it himself */
|
/* since the caller is responsible for closing it himself */
|
||||||
FT_FREE( stream );
|
|
||||||
stream = args->stream;
|
stream = args->stream;
|
||||||
|
error = FT_Err_Ok;
|
||||||
}
|
}
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
else
|
else
|
||||||
|
{
|
||||||
error = FT_THROW( Invalid_Argument );
|
error = FT_THROW( Invalid_Argument );
|
||||||
|
if ( ( args->flags & FT_OPEN_STREAM ) && args->stream )
|
||||||
|
FT_Stream_Close( args->stream );
|
||||||
|
}
|
||||||
|
|
||||||
if ( error )
|
if ( !error )
|
||||||
FT_FREE( stream );
|
{
|
||||||
else
|
stream->memory = memory;
|
||||||
stream->memory = memory; /* just to be certain */
|
*astream = stream;
|
||||||
|
}
|
||||||
*astream = stream;
|
|
||||||
|
|
||||||
Exit:
|
Exit:
|
||||||
return error;
|
return error;
|
||||||
|
|
Loading…
Reference in New Issue