Hello,
On Wed, Oct 14, 2026 at 09:16:58PM +0900, Dongwon Jang wrote:
> Since commit 3ef263987 ("MEDIUM: chunks: Realloc trash buffers only after
> the config is parsed and checked") in 1.8, the trash keeps its early
> default size (BUFSIZE) until init_trash_buffers() is called after
> check_config_validity(). Pattern files (maps, ACL "-f" files and
> set-vars-from-map files) are loaded before that point, and their lines are
> read with fgets() into the trash. As a result, a line longer than BUFSIZE-1
> (16383 by default) is silently split regardless of tune.bufsize: the first
> part is loaded with a truncated value and the remainder is loaded as one or
> more extra entries, without any warning. For ACLs, this also means that the
> 16383-char prefix of a long pattern matches while the full pattern does
> not. The only workarounds are to shorten such lines or to set the entries
> again from the CLI after each reload.
>
> Let's read pattern file lines into a dedicated heap buffer that grows as
> needed, as readcfgfile() does for configuration lines. getline() is not
> used since it was removed for portability reasons by commit fbc3d46b9
> ("BUILD: stats: remove non portable getline() usage"), and resizing the
> trash earlier was not an option since ACL files are loaded while parsing.
> Values remain bounded at runtime by tune.bufsize since pattern_exec_match()
> copies the matched sample into a trash chunk, which is consistent with what
> can be set from the CLI.
>
> A reg-test was added which checks that a map file and an ACL file holding
> a single 20000-char line are each loaded as a single entry, and that the
> map value is returned in full.
Interesting!
> This should be backported to all stable versions. Before 3.0, the two
> pat_ref_read_from_file*() functions take the file name as an argument,
> which only changes the patch context.
> ---
> I hit this on 3.2.21 with a map holding base64-encoded response bodies
> returned by "http-request return", which were served truncated after each
> reload. It is still present in master and in the 3.2, 3.3 and 3.4 branches.
>
> Tested on vanilla 3.2.21, 3.4.6 and master, on macOS (clang 17) and on
> Linux x86_64 (Rocky 9, gcc 11.5): the new reg-test fails without the fix
> and passes with it, and regular map files load exactly as before. On
> master, the existing map/ACL reg-tests and the full suite give the same
> results as without the patch, and loading a 2M-line map takes the same
> time.
>
> I'll open a GitHub issue pointing to this thread. Please keep me CCed,
> I'm not subscribed to the list.
OK. I'm just having a request though:
> +/* Reads the next line of pattern file <file> into <*buf>, whose allocated
> + * size is <*size>, growing the buffer as needed. Lines are not read into the
> + * trash because pattern files are loaded while the configuration is parsed
> and
> + * checked, when the trash still has its early default size (tune.bufsize is
> + * only applied afterwards), so longer lines used to be silently split into
> + * several entries. <*buf> must either be NULL, in which case it is
> allocated,
> + * or point to the area of <*size> bytes it was given by a previous call, and
> + * it must be released by the caller with free(). Returns 1 if a line was
> read,
> + * 0 at the end of the file or on read error, and -1 on memory allocation
> + * failure.
> + */
> +static int pat_ref_read_line(FILE *file, char **buf, size_t *size)
> +{
> + size_t len = 0;
> + char *new_buf;
> +
> + if (!*buf) {
> + *buf = malloc(LINESIZE);
> + if (!*buf)
> + return -1;
> + *size = LINESIZE;
> + }
> +
> + while (fgets(*buf + len, *size - len, file) != NULL) {
> + len += strlen(*buf + len);
> + if (len < *size - 1 || (*buf)[len - 1] == '\n')
> + return 1;
> +
> + /* the buffer is full and the line is not complete yet */
> + new_buf = realloc(*buf, *size * 2);
> + if (!new_buf)
> + return -1;
> + *buf = new_buf;
> + *size *= 2;
> + }
> + return len ? 1 : 0;
> +}
This function is not specific to patterns at all and should move to
tools.c under a different name. It reads a line into a caller-provided
memory area, so how about read_line_to_mem() ? You could place it just
after read_line_to_trash() which is very close, so that developers meet
the two together when they're searching for something related.
Please move that function in a first patch (one tagged minor that adds
the new function) and just mention in the second (the bug fix) that it
depends on this one for backports.
Otherwise your patch looks valid at first glance.
Thanks!
Willy
