On Tue, 26 Aug 2025, Nicolas George via ffmpeg-devel wrote:
Marton Balint via ffmpeg-devel (HE12025-08-24):
From 4532caf820dcb2bc2cbac3d4e782ad27cf9fd76b Mon Sep 17 00:00:00 2001
From: Marton Balint <c...@passwd.hu>
Date: Sun, 24 Aug 2025 21:35:41 +0200
Subject: [PATCH 2/2] avutil/bprint: fix av_bprint_strftime with %p format
string reporting truncated output
strftime returns 0 in case of an empty output (e.g. %p format string with some
locales), there is no way to distinguish this from a buffer-too-small error
condition. So we must use some heuristics to handle this case, and not consume
INT_MAX RAM and falsely report a truncated output.
Signed-off-by: Marton Balint <c...@passwd.hu>
---
libavutil/bprint.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/libavutil/bprint.c b/libavutil/bprint.c
index 932c03ce50..fa244b2e04 100644
--- a/libavutil/bprint.c
+++ b/libavutil/bprint.c
@@ -167,6 +167,7 @@ void av_bprint_strftime(AVBPrint *buf, const char *fmt,
const struct tm *tm)
{
unsigned room;
size_t l;
+ size_t fmt_len = 0;
Why not set it here once and for all?
I just thought that for the typical case we can spare an strlen() call
this way.
if (!*fmt)
return;
@@ -174,9 +175,16 @@ void av_bprint_strftime(AVBPrint *buf, const char *fmt,
const struct tm *tm)
room = av_bprint_room(buf);
if (room && (l = strftime(buf->str + buf->len, room, fmt, tm)))
break;
+
+ if (!fmt_len)
+ fmt_len = strlen(fmt);
+ /* A 256x space requirement for output is unlikely, so it is likely
empty */
+ if (room >> 8 > fmt_len)
+ break;
+
IIUC, at this point l=0 means either the buffer is too small or the
output is empty, and you distinguish between the cases by checking that
there is plenty of room, 256 chars for any char in the buffer?
Yes.
I think a lower margin should be ok, >>4 or >>5.
I'd rather keep it >>8, because strftime format strings can also
have width specifiers, so something like "%80c" should work. Obviously
this is always going to be a heuristics, but I wanted to be careful and
reject only the very unlikely/insane format strings...
OTOH, I think a comment would be a good idea.
There is one, but OK, I will make the existing comment a bit more
verbose.
/* strftime does not tell us how much room it would need: let us
retry with twice as much until the buffer is large enough */
- room = !room ? strlen(fmt) + 1 :
+ room = !room ? fmt_len + 1 :
room <= INT_MAX / 2 ? room * 2 : INT_MAX;
if (av_bprint_alloc(buf, room)) {
/* impossible to grow, try to manage something useful anyway */
Or we could write our own strftime, not localized.
Some API users might depend on our strftime functions returning localized
strings...
Regards,
Marton
Regards,
--
Nicolas George
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-le...@ffmpeg.org
_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-le...@ffmpeg.org
