Skip to content

out_s3: add time offset support for s3 key format #10752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

out_s3: add time offset support for s3 key format #10752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion include/fluent-bit/flb_aws_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ int flb_read_file(const char *path, char **out_buf, size_t *out_size);

/* Constructs S3 object key as per the format. */
flb_sds_t flb_get_s3_key(const char *format, time_t time, const char *tag,
char *tag_delimiter, uint64_t seq_index);
char *tag_delimiter, uint64_t seq_index, const char *time_offset);

/* Constructs S3 object key as per the blob format. */
flb_sds_t flb_get_s3_blob_key(const char *format, const char *tag,
Expand All @@ -208,5 +208,12 @@ flb_sds_t flb_get_s3_blob_key(const char *format, const char *tag,
size_t flb_aws_strftime_precision(char **out_buf, const char *time_format,
struct flb_time *tms);


/*
* Parses the time offset and returns the offset in seconds.
*/
size_t flb_aws_parse_tz_offset(const char *time_offset);

Comment on lines +211 to +216
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Bug: flb_aws_parse_tz_offset must return a signed type

Returning size_t breaks negative offsets (wrap-around to huge positive), potentially corrupting timestamps. Use a signed integer type (int is sufficient; range ±86399).

Apply this header change:

-/*
- * Parses the time offset and returns the offset in seconds.
- */
-size_t flb_aws_parse_tz_offset(const char *time_offset);
+/*
+ * Parses the time offset and returns the offset in seconds (signed).
+ * Returns 0 on invalid input.
+ */
+int flb_aws_parse_tz_offset(const char *time_offset);

And align the implementation in src/aws/flb_aws_util.c:

-size_t flb_aws_parse_tz_offset(const char *time_offset) {
+int flb_aws_parse_tz_offset(const char *time_offset) {
     int sign, hh, mm;
@@
-    return sign * (hh * 3600 + mm * 60);
+    return sign * (hh * 3600 + mm * 60);
 }

Also ensure flb_get_s3_key uses a signed type when applying the offset to time_t:

/* Example inside flb_get_s3_key */
int tz_offset = flb_aws_parse_tz_offset(time_offset);
if (tz_offset != 0) {
    base_time += tz_offset;
}
🤖 Prompt for AI Agents 
In include/fluent-bit/flb_aws_util.h around lines 211-216, change the prototype
return type of flb_aws_parse_tz_offset from size_t to a signed type (int) so
negative timezone offsets are represented correctly; update the implementation
in src/aws/flb_aws_util.c to return the same signed type and ensure all internal
math uses signed arithmetic; finally update callers such as flb_get_s3_key to
store the return in an int (e.g., int tz_offset =
flb_aws_parse_tz_offset(time_offset)) and apply that signed offset to time_t
(base_time += tz_offset) to avoid unsigned wraparound.


#endif
#endif /* FLB_HAVE_AWS */
12 changes: 10 additions & 2 deletions plugins/out_s3/s3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1460,7 +1460,7 @@ static int s3_put_object(struct flb_s3 *ctx, const char *tag, time_t file_first_
char final_body_md5[25];

s3_key = flb_get_s3_key(ctx->s3_key_format, file_first_log_time, tag,
ctx->tag_delimiters, ctx->seq_index);
ctx->tag_delimiters, ctx->seq_index, ctx->time_offset);
if (!s3_key) {
flb_plg_error(ctx->ins, "Failed to construct S3 Object Key for %s", tag);
return -1;
Expand Down Expand Up @@ -1648,7 +1648,7 @@ static struct multipart_upload *create_upload(struct flb_s3 *ctx, const char *ta
return NULL;
}
s3_key = flb_get_s3_key(ctx->s3_key_format, file_first_log_time, tag,
ctx->tag_delimiters, ctx->seq_index);
ctx->tag_delimiters, ctx->seq_index, ctx->time_offset);
if (!s3_key) {
flb_plg_error(ctx->ins, "Failed to construct S3 Object Key for %s", tag);
flb_free(m_upload);
Expand Down Expand Up @@ -4034,6 +4034,14 @@ static struct flb_config_map config_map[] = {
"documentation."
},

{
FLB_CONFIG_MAP_STR, "time_offset", "+0000",
0, FLB_TRUE, offsetof(struct flb_s3, time_offset),
"Time offset to apply to the s3 key format. This is useful when the s3 key format is "
"in a different timezone than the S3 bucket. The format is +HHMM or -HHMM. "
"Default is +0000."
},

{
FLB_CONFIG_MAP_STR, "s3_key_format_tag_delimiters", ".",
0, FLB_TRUE, offsetof(struct flb_s3, tag_delimiters),
Expand Down
3 changes: 2 additions & 1 deletion plugins/out_s3/s3.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ struct flb_s3 {
char *bucket;
char *region;
char *s3_key_format;
char *time_offset;
char *tag_delimiters;
char *endpoint;
char *sts_endpoint;
Expand All @@ -113,7 +114,7 @@ struct flb_s3 {
char *storage_class;
char *log_key;
char *external_id;
char *profile;
char *profile;
int free_endpoint;
int retry_requests;
int use_put_object;
Expand Down
46 changes: 45 additions & 1 deletion src/aws/flb_aws_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@
#include <fluent-bit/flb_jsmn.h>
#include <fluent-bit/flb_env.h>

#include <stddef.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <ctype.h>

#define AWS_SERVICE_ENDPOINT_FORMAT "%s.%s.amazonaws.com"
#define AWS_SERVICE_ENDPOINT_BASE_LEN 15
Expand Down Expand Up @@ -1009,7 +1011,7 @@ flb_sds_t flb_get_s3_blob_key(const char *format,

/* Constructs S3 object key as per the format. */
flb_sds_t flb_get_s3_key(const char *format, time_t time, const char *tag,
char *tag_delimiter, uint64_t seq_index)
char *tag_delimiter, uint64_t seq_index, const char *time_offset)
{
int i = 0;
int ret = 0;
Expand All @@ -1027,6 +1029,13 @@ flb_sds_t flb_get_s3_key(const char *format, time_t time, const char *tag,
flb_sds_t tmp_key = NULL;
flb_sds_t tmp_tag = NULL;
struct tm gmt = {0};
size_t tz_offset = 0;

tz_offset = flb_aws_parse_tz_offset(time_offset);

if (tz_offset != 0) {
time += tz_offset;
}
Comment on lines +1032 to +1038
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Bug: negative offsets break due to unsigned type; use a signed type for tz offset

tz_offset is declared as size_t, but flb_aws_parse_tz_offset can return negative seconds for "-HHMM". Assigning a negative value to size_t causes wrap-around to a huge positive number, producing a wildly incorrect timestamp when added to time. Use a signed type (int or time_t) for both the function return and the local variable.

Apply this diff:

-    size_t tz_offset = 0;
+    int tz_offset = 0;
@@
-    tz_offset = flb_aws_parse_tz_offset(time_offset);
+    tz_offset = flb_aws_parse_tz_offset(time_offset);
@@
-    if (tz_offset != 0) {
-        time += tz_offset;
-    }
+    if (tz_offset != 0) {
+        time += tz_offset;
+    }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/aws/flb_aws_util.c around lines 1032 to 1038, tz_offset is declared as
size_t but flb_aws_parse_tz_offset can return negative values; change the local
tz_offset to a signed type (e.g., long or time_t/int) and update
flb_aws_parse_tz_offset's return type to the same signed type so negative
offsets are preserved; after changing types, adjust any callers/assignments and
ensure arithmetic on time uses the signed tz_offset to correctly add/subtract
negative offsets and recompile to satisfy type changes.


if (strlen(format) > S3_KEY_SIZE){
flb_warn("[s3_key] Object key length is longer than the 1024 character limit.");
Expand Down Expand Up @@ -1324,3 +1333,38 @@ size_t flb_aws_strftime_precision(char **out_buf, const char *time_format,

return out_size;
}


size_t flb_aws_parse_tz_offset(const char *time_offset) {
int sign, hh, mm;

if (!time_offset) {
return 0;
}

if (strlen(time_offset) < 5) {
return 0;
}

if (time_offset[0] != '+' && time_offset[0] != '-') {
return 0;
}

sign = (time_offset[0] == '+') ? 1 : -1;

if (!isdigit(time_offset[1]) ||
!isdigit(time_offset[2]) ||
!isdigit(time_offset[3]) ||
!isdigit(time_offset[4])) {
return 0;
}

hh = ((time_offset[1] - '0') * 10) + (time_offset[2] - '0');
mm = ((time_offset[3] - '0') * 10) + (time_offset[4] - '0');

if (hh < 0 || hh > 23 || mm < 0 || mm > 59) {
return 0;
}

return sign * (hh * 3600 + mm * 60);
}
Comment on lines +1338 to +1370
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Incorrect return type for offset parser; tighten validation and avoid ctype UB

flb_aws_parse_tz_offset returns size_t but computes signed offsets. This will misrepresent negative offsets. Also, accept exactly 5 characters (±HHMM) and cast to unsigned char for isdigit to avoid undefined behavior.

Apply this diff:

-size_t flb_aws_parse_tz_offset(const char *time_offset) {
+int flb_aws_parse_tz_offset(const char *time_offset) {
     int sign, hh, mm;
 
     if (!time_offset) {
         return 0;
     }
 
-    if (strlen(time_offset) < 5) {
+    /* Expect exactly 5 characters: ±HHMM */
+    if (strlen(time_offset) != 5) {
         return 0;
     }
 
     if (time_offset[0] != '+' && time_offset[0] != '-') {
         return 0;
     }
 
     sign = (time_offset[0] == '+') ? 1 : -1;
 
-    if (!isdigit(time_offset[1]) || 
-        !isdigit(time_offset[2]) || 
-        !isdigit(time_offset[3]) || 
-        !isdigit(time_offset[4])) {
+    if (!isdigit((unsigned char) time_offset[1]) ||
+        !isdigit((unsigned char) time_offset[2]) ||
+        !isdigit((unsigned char) time_offset[3]) ||
+        !isdigit((unsigned char) time_offset[4])) {
         return 0;
     }
 
     hh = ((time_offset[1] - '0') * 10) + (time_offset[2] - '0');
     mm = ((time_offset[3] - '0') * 10) + (time_offset[4] - '0');
 
     if (hh < 0 || hh > 23 || mm < 0 || mm > 59) {
         return 0;
     }
 
     return sign * (hh * 3600 + mm * 60);
 }

Optional: consider logging a warning when an invalid string is provided so users know their configured offset was ignored.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
size_t flb_aws_parse_tz_offset(const char *time_offset) {
int sign, hh, mm;
if (!time_offset) {
return 0;
}
if (strlen(time_offset) < 5) {
return 0;
}
if (time_offset[0] != '+' && time_offset[0] != '-') {
return 0;
}
sign = (time_offset[0] == '+') ? 1 : -1;
if (!isdigit(time_offset[1]) ||
!isdigit(time_offset[2]) ||
!isdigit(time_offset[3]) ||
!isdigit(time_offset[4])) {
return 0;
}
hh = ((time_offset[1] - '0') * 10) + (time_offset[2] - '0');
mm = ((time_offset[3] - '0') * 10) + (time_offset[4] - '0');
if (hh < 0 || hh > 23 || mm < 0 || mm > 59) {
return 0;
}
return sign * (hh * 3600 + mm * 60);
}
int flb_aws_parse_tz_offset(const char *time_offset) {
int sign, hh, mm;
if (!time_offset) {
return 0;
}
/* Expect exactly 5 characters: ±HHMM */
if (strlen(time_offset) != 5) {
return 0;
}
if (time_offset[0] != '+' && time_offset[0] != '-') {
return 0;
}
sign = (time_offset[0] == '+') ? 1 : -1;
if (!isdigit((unsigned char)time_offset[1]) ||
!isdigit((unsigned char)time_offset[2]) ||
!isdigit((unsigned char)time_offset[3]) ||
!isdigit((unsigned char)time_offset[4])) {
return 0;
}
hh = ((time_offset[1] - '0') * 10) + (time_offset[2] - '0');
mm = ((time_offset[3] - '0') * 10) + (time_offset[4] - '0');
if (hh < 0 || hh > 23 || mm < 0 || mm > 59) {
return 0;
}
return sign * (hh * 3600 + mm * 60);
}
🤖 Prompt for AI Agents 
In src/aws/flb_aws_util.c around lines 1338 to 1370, flb_aws_parse_tz_offset
currently returns size_t and performs isdigit calls on plain char and allows
strings shorter than 5; change the function to return a signed integer type
(e.g. int or long) so negative offsets are represented correctly, require the
input length to be exactly 5 characters (format ±HHMM), cast characters to
(unsigned char) before calling isdigit to avoid ctype UB, and keep the same
numeric validation for HH (0-23) and MM (0-59) while returning sign * (hh*3600 +
mm*60); optionally add a warning log when an invalid string is provided.

Loading