Modules: CR/LF in njs-produced nginx variables #1127

#1111 added header-value validation to r.headersOut and to redirect targets passed through r.return().

In the review for #1111, the remaining raw conversion in ngx_http_js_request_variables() was noted as:

not a header path -- correctly left alone.

There is a consequence of that distinction when an njs-produced variable is later consumed by a directive that serializes it into an HTTP header.

The shortest case uses js_set:

function v(r) {
    return r.args.v;
}

export default { v };

js_import test from test.js;
js_set $fromjs test.v;

location /set {
    add_header X-Test $fromjs;
    return 200 "ok\n";
}

Requesting:

/set?v=a%0d%0aInjected:%201

produces:

X-Test: a
Injected: 1

The value is already decoded when the handler sees it. Reporting the character codes of r.args.v for the same request gives:

97,13,10,...

so CR and LF are present as actual characters rather than as the encoded %0d%0a text.

The equivalent direct header store is rejected:

r.headersOut['X-Test'] = r.args.v;
// TypeError: invalid header value

while both of these preserve the CR/LF:

// js_set
return r.args.v;

// r.variables
r.variables.injected = r.args.v;

The r.variables case can also reach an upstream request header:

function up(r) {
    r.variables.injected = r.args.v;
    r.internalRedirect('/proxied');
}

export default { up };

js_import test from test.js;
js_var $injected;

location /up {
    js_content test.up;
}

location /proxied {
    internal;
    proxy_set_header X-Fwd $injected;
    proxy_pass http://127.0.0.1:9002;
}

Requesting:

/up?v=a%0d%0aX-Smuggled:%20yes

makes nginx send upstream:

GET /proxied HTTP/1.0
X-Fwd: a
X-Smuggled: yes
Host: 127.0.0.1:9002
Connection: close

This behavior is not unique to njs variables.

For example, $arg_v is not decoded:

add_header X-Test $arg_v;

and therefore produces:

X-Test: a%0d%0aInjected:%201

On the other hand, nginx variables can themselves contain decoded data. For example, $uri can produce the same header splitting behavior when used with add_header, with no njs involved.

So I am not assuming that generic nginx variables are required to satisfy HTTP header-value syntax.

The njs-specific case is that request data such as r.args.v is exposed decoded, and js_set or r.variables can carry that value into a generic nginx variable which is then consumed by a header-producing directive.

Given the distinction made in #1111, is preserving CR/LF in njs-produced nginx variables considered intentional even when those variables are later consumed by directives such as add_header or proxy_set_header?

As a sanity check, I locally tested rejecting only CR and LF at the points where an njs value becomes an nginx variable, including both js_set and r.variables.

With that change:

• the js_set example no longer emits X-Test;
• assigning the same value through r.variables throws TypeError: invalid variable value;
• the upstream-header case is no longer possible;
• ordinary values remain unchanged, including plain, UTF-8 such as café, and values containing a tab.

I limited the test to CR and LF rather than applying the full ngx_js_check_header_value() rules, since nginx variables are generic and may be consumed outside HTTP headers.

Tested with njs 1.0.1 built into nginx 1.28.0, using both the default engine and js_engine qjs; the behavior is identical with both.

I also ran the 104 nginx/t js and stream_js test files (1003 assertions) against the changed and unchanged builds. The pass/fail sets were identical. Twelve files fail in both builds in my ASan -j4 environment, so there was no regression delta from this change.