Skip to content
This repository was archived by the owner on Oct 8, 2025. It is now read-only.

Fix missing newlines in access logs for JS configuration #1459

Merged
merged 2 commits into from
Oct 23, 2024
Merged

Fix missing newlines in access logs for JS configuration #1459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de430ed
Add flag for newline control in access log entries
hongzhidao Oct 14, 2024
76cc071
Fix missing newlines in access logs for JS configuration
hongzhidao Oct 14, 2024
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
18 changes: 14 additions & 4 deletions src/nxt_js.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ nxt_js_add_module(nxt_js_conf_t *jcf, nxt_str_t *name, nxt_str_t *text)


nxt_js_t *
nxt_js_add_tpl(nxt_js_conf_t *jcf, nxt_str_t *str, nxt_bool_t strz)
nxt_js_add_tpl(nxt_js_conf_t *jcf, nxt_str_t *str, nxt_uint_t flags)
{
size_t size;
u_char *p, *start;
Expand All @@ -243,13 +243,19 @@ nxt_js_add_tpl(nxt_js_conf_t *jcf, nxt_str_t *str, nxt_bool_t strz)
" return ");

/*
* Appending a terminating null character if strz is true.
* Append a newline character if newline is true.
* Append a terminating null character if strz is true.
*/
static const nxt_str_t newline_str = nxt_string(" + '\\x0A'");
static const nxt_str_t strz_str = nxt_string(" + '\\x00'");

size = func_str.length + str->length + 1;

if (strz) {
if (flags & NXT_TSTR_NEWLINE) {
size += newline_str.length;
}

if (flags & NXT_TSTR_STRZ) {
size += strz_str.length;
}

Expand All @@ -263,7 +269,11 @@ nxt_js_add_tpl(nxt_js_conf_t *jcf, nxt_str_t *str, nxt_bool_t strz)
p = nxt_cpymem(p, func_str.start, func_str.length);
p = nxt_cpymem(p, str->start, str->length);

if (strz) {
if (flags & NXT_TSTR_NEWLINE) {
p = nxt_cpymem(p, newline_str.start, newline_str.length);
}

if (flags & NXT_TSTR_STRZ) {
p = nxt_cpymem(p, strz_str.start, strz_str.length);
}

Expand Down
2 changes: 1 addition & 1 deletion src/nxt_js.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ void nxt_js_conf_release(nxt_js_conf_t *jcf);
void nxt_js_set_proto(nxt_js_conf_t *jcf, njs_external_t *proto, nxt_uint_t n);
nxt_int_t nxt_js_add_module(nxt_js_conf_t *jcf, nxt_str_t *name,
nxt_str_t *text);
nxt_js_t *nxt_js_add_tpl(nxt_js_conf_t *jcf, nxt_str_t *str, nxt_bool_t strz);
nxt_js_t *nxt_js_add_tpl(nxt_js_conf_t *jcf, nxt_str_t *str, nxt_uint_t flags);
nxt_int_t nxt_js_compile(nxt_js_conf_t *jcf);
nxt_int_t nxt_js_test(nxt_js_conf_t *jcf, nxt_str_t *str, u_char *error);
nxt_int_t nxt_js_call(nxt_task_t *task, nxt_js_conf_t *jcf,
Expand Down
15 changes: 2 additions & 13 deletions src/nxt_router_access_log.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ nxt_int_t
nxt_router_access_log_create(nxt_task_t *task, nxt_router_conf_t *rtcf,
nxt_conf_value_t *value)
{
u_char *p;
nxt_int_t ret;
nxt_str_t str;
nxt_tstr_t *format;
Expand Down Expand Up @@ -121,18 +120,8 @@ nxt_router_access_log_create(nxt_task_t *task, nxt_router_conf_t *rtcf,
nxt_memcpy(access_log->path.start, alcf.path.start, alcf.path.length);
}

str.length = alcf.format.length + 1;

str.start = nxt_malloc(str.length);
if (str.start == NULL) {
nxt_alert(task, "failed to allocate log format structure");
return NXT_ERROR;
}

p = nxt_cpymem(str.start, alcf.format.start, alcf.format.length);
*p = '\n';

format = nxt_tstr_compile(rtcf->tstr_state, &str, NXT_TSTR_LOGGING);
format = nxt_tstr_compile(rtcf->tstr_state, &alcf.format,
NXT_TSTR_LOGGING | NXT_TSTR_NEWLINE);
if (nxt_slow_path(format == NULL)) {
return NXT_ERROR;
}
Expand Down
11 changes: 8 additions & 3 deletions src/nxt_tstr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,17 @@ nxt_tstr_compile(nxt_tstr_state_t *state, const nxt_str_t *str,
{
u_char *p;
nxt_tstr_t *tstr;
nxt_bool_t strz;
nxt_bool_t strz, newline;

strz = (flags & NXT_TSTR_STRZ) != 0;
newline = (flags & NXT_TSTR_NEWLINE) != 0;
Comment on lines 85 to +86
Copy link
Member

Choose a reason for hiding this comment

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

I don't know why we do the test like this (!= 0) it can only be either set or not.

We don't do it like that in the if () statements...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense.

Copy link
Contributor Author

@hongzhidao hongzhidao Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We missed something here. Because of the calculation below:

 tstr->str.length = str->length + newline + strz;

tstr should be either 0 or 1.
(flags & NXT_TSTR_NEWLINE) may be greater than 1 but (flags & NXT_TSTR_NEWLINE) != 0 not.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, indeed... sorry brain fart on my part...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries, tests can help in this case.


tstr = nxt_mp_get(state->pool, sizeof(nxt_tstr_t));
if (nxt_slow_path(tstr == NULL)) {
return NULL;
}

tstr->str.length = str->length + strz;
tstr->str.length = str->length + newline + strz;

tstr->str.start = nxt_mp_nget(state->pool, tstr->str.length);
if (nxt_slow_path(tstr->str.start == NULL)) {
Expand All @@ -98,6 +99,10 @@ nxt_tstr_compile(nxt_tstr_state_t *state, const nxt_str_t *str,

p = nxt_cpymem(tstr->str.start, str->start, str->length);

if (newline) {
*p++ = '\n';
}

if (strz) {
*p = '\0';
}
Expand All @@ -114,7 +119,7 @@ nxt_tstr_compile(nxt_tstr_state_t *state, const nxt_str_t *str,

nxt_tstr_str(tstr, &tpl);

tstr->u.js = nxt_js_add_tpl(state->jcf, &tpl, strz);
tstr->u.js = nxt_js_add_tpl(state->jcf, &tpl, flags);
if (nxt_slow_path(tstr->u.js == NULL)) {
return NULL;
}
Expand Down
1 change: 1 addition & 0 deletions src/nxt_tstr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ typedef struct {
typedef enum {
NXT_TSTR_STRZ = 1 << 0,
NXT_TSTR_LOGGING = 1 << 1,
NXT_TSTR_NEWLINE = 1 << 2,
} nxt_tstr_flags_t;


Expand Down
Loading