nginx/njs

If two or more "location" block defines the same "js_var" directive name, the value will be overwritten

Closed this issue · 15 comments

Abstract:

If two or more "location" block defines the same "js_var" directive name, the value in the previous "location" block will be overwritten by the next "location" block. This behaviour is not the same as the "set" directive, it should be consider as a bug.


Detail:

js_content directive can pass "module.function" which only accept one paramter (r: NginxHTTPRequest), but sometimes developer want to pass more paramters by location block with same "module.function". js_var or set may be used as this idea.

The problem is, when use the same name in js_var, the last one will overwrite the previous one. that means value isolation by location block fail.

This behavior does not exist in the "set" directive, so it should be consider as a bug.


Environment & Version:

CentOS 7
nginx.x86_64 1:1.25.2-1.el7.ngx
nginx-module-njs.x86_64 1:1.25.2+0.8.0-1.el7.ngx


Steps to reproduce:

nginx.conf:

server {
    listen       8765;
    server_name  localhost;
    

    js_import test from test.js;

    location /test {
        js_var $js_var_test "js_var_test from /test";
        set $set_test "set_test from /test";
        js_content test.test;
    }

    location /test2 {
        js_var $js_var_test "js_var_test from /test2";
        set $set_test "set_test from /test2";
        js_content test.test;
    }

}

test.js:

function test(r){
    r.return(200, JSON.stringify({
        "js_var_test": r.variables.js_var_test,
        "set_test": r.variables.set_test
    }));
}


export default {test};

curl /test:

curl -v "http://localhost:8765/test"
curl /test2:

curl -v "http://localhost:8765/test2"


What is expected?

Started from "from" string, the value from js_var_test should be the same as set_test.


What is actually happening?

curl /test:

{"js_var_test":"js_var_test from /test2","set_test":"set_test from /test"}
(js_var_test is overwritten)

curl /test2:

{"js_var_test":"js_var_test from /test2","set_test":"set_test from /test2"}

xeioex commented

Hi @HorseLuke,

Thank you for your report.

js_set is specifically implemented in a different way to set. If they are the same we do not need both.

They are similar and the main difference in that js_set is not overwritten after r.internalRedirect() to a new location.
This is exactly what you are seeing with set variables.

For example:

    location /test {
        # when you hit /test location the value for $set_test is calculated
        # in a rewrite phase, every time your request hits the location regardless
        # of njs
        set $set_test "AAA";
    }

    location /test2 {
        # when you hit /test location the value for $set_test is calculated
        # in a rewrite phase, every time your request hits the location regardless
        # of njs
        set $set_test "BBB";
    }
    
    # both $set_test are the same variable, but populated by different init code depending on a location
        

js_var on the other hand is never evaluated until it is first accessed by njs or other directives.
NJS looks for a variable by name using ngx_http_get_variable. Which looks for a variable by name in a global variable hash.

In summary: I cannot use the same init mechanism that works on rewrite phase which set is using, because it will break the indented semantic with internal redirects. Without it I do not see a good solution for location matching during a variable evaluation.

Sorry for delayed response.

So if I want to use same "module.function" with js_content directive in difference location block, how to pass difference arguments for "module.function"?

xeioex commented

Hi @HorseLuke,

Please, elaborate more on what you are trying to achieve. Does set solve the problem or not?

Now I use global set to solve my problem, the solution is added in #668 (comment) .

But I'd like to share why I opened those 2 issues.

My scenario is simple, use njs as a auth gateway to auth client token, if auth success then pass to backend with authencated userinfo, if not then show custom error to client.

You may wonder why not use auth_request(then use js_content in it) , because:

(1) if auth_request fail, it can not output custom error info to client by fail condition. For example, if token expired then output "token_exipred" , if user is banned then output "user_banned". Now auth_request only return 401 httpcode without custom info, and can not pass error info to 401 httpcode location.

(2) if auth_request success, pass success info (e.g., authencated userinfo) to upstream is not in natural way. The only way for js_content in auth_request is to use headerOut.

If not use auth_request, another way is to use named location like this:

nginx.conf

server {

  set $param_js_content_param_named_location "@not_exists";

  js_var $data_from_js_content_auth_user_info "";

  js_import main from ../dist/main.js;

    //..omit...


  location = /testapp/v1/get{
    set $param_js_content_param_named_location "@upstream_testapp1";
    js_content main.doAuthAndRedirect;
  }

  location = /testapp2/v1/get{
    set $param_js_content_param_named_location "@upstream_testapp2";
    js_content main.doAuthAndRedirect;
  }

  location @upstream_testapp1{
     proxy_pass  http://127.0.0.1:6871;
     proxy_set_header X-Authed-User-Info $data_from_js_content_auth_user_info;
    //..omit...
  }

  location @upstream_testapp2{
     proxy_pass  http://127.0.0.1:7892;
     proxy_set_header X-Authed-User-Info $data_from_js_content_auth_user_info;
    //..omit...

  }

  location @not_exists{
    //..omit...
  }

}

(If auth success, js_content will write auth userinfo to r.variables["data_from_js_content_auth_user_info"], then use r.internalRedirect(r.variables["param_js_content_param_named_location"]))

You can see that every location has the same js_content with difference param, which trying to solve js_content can not pass extra param problem. But at the first time I encounter problem no matter I use js_var or set , and the behavior is like bugs, so I opened these 2 issues. Now it seems it is by-design, I find a way to pass it.

Closed as "by-design"

(1) if auth_request fail, it can not output custom error info to client by fail condition. For example, if token expired then output "token_exipred" , if user is banned then output "user_banned". Now auth_request only return 401 httpcode without custom info, and can not pass error info to 401 httpcode location.

Not true. auth_request_set can be used to return metadata from an auth_request/js_content location that can then be used as part of an error_page location.

Does that help?

(1) if auth_request fail, it can not output custom error info to client by fail condition. For example, if token expired then output "token_exipred" , if user is banned then output "user_banned". Now auth_request only return 401 httpcode without custom info, and can not pass error info to 401 httpcode location.

Not true. auth_request_set can be used to return metadata from an auth_request/js_content location that can then be used as part of an error_page location.

Does that help?

Not completely true.
I mean that I'd like to have full control of fail process output, not only the body response but also http code (like banned 403, auth server error 500, not always 401), custom response header (like add quota limit info header), etc. auth_request can not do that. auth_request_set is not a straight solution.

auth_request can not do that

It can when combined with error_page. With js_content you are in control of the response code from auth_request and so you can return any response you wish as a result of the code, headers or body of the subrequest.

drsm commented

@HorseLuke

Take a look , #362 (comment)

Thanks for sharing. In fact that is the first version I implemented (almost, but without custom body and http code at that time because of not knowing how to use auth_request_set).
But as what I said above, passing variables across block directive will add more complexity of the code, and iti is very difficult to maintain when in production env. When encounter bugs, you have to jump between nginx config file and njs module, teammates will not happy and hate it. So that is not a straight solution.

auth_request can not do that

It can when combined with error_page. With js_content you are in control of the response code from auth_request and so you can return any response you wish as a result of the code, headers or body of the subrequest.

Emmm......Maybe we are not in the same topic.

My topic is to take full control in njs module as much as possible, and not to jump across config file and njs module. This will make test easier, and reuse js code easier.

What you said is to split functions into config blocks. I tried it, and my conclusion is that it add complexity and not easy to test and maintain.

I understand your goal. In general, it's better to use native nginx functionality when it's available. It does add complexity. Some of that can be helped with include files and clear naming of functions and locations, but you get better performance as a result. It's a tradeoff.

Hi @lcrilly are you really sure? I'm facing the same issue are you really sure that it's possible? My setup is like that:

nginx.conf

        error_page 500 @process_error_500;

        location / {
            auth_request /__api__acces_check$is_args$args;
            ....
        }
        
        location /__api__acces_check {
            internal;
            js_content auth.access_chech;
        }
        
        location @process_error_500 {
            js_content auth.error_page500;
        }

js

function access_chech(request)
{
    request.return(500, 'YOLO');
}

function error_page500(request)
{
    request.return(503, "Whatever");
}

I can change the BODY response but the status code is always 500 :/

< HTTP/1.1 500 Internal Server Error
< Server: nginx/1.25.4
< Date: Mon, 08 Apr 2024 07:58:58 GMT
< Content-Type: application/json
< Content-Length: 8
< Connection: close
< x-original-status: 500
< Access-Control-Allow-Origin: *
< Access-Control-Allow-Methods: POST, GET, OPTIONS, PUT, DELETE
< Allow: POST, GET, OPTIONS, PUT, DELETE
< Access-Control-Allow-Headers: x-auth-token,content-type,x-auth-tenant,Authorization,x-user-scope
< X-Content-Type-Options: nosniff
< X-Frame-Options: SAMEORIGIN
< X-XSS-Protection: 1; mode=block
< X-Content-Security-Policy: allow 'self'
< 
* Closing connection
Whatever

@salacr please try using = with the error_page directive with one of these:

error_page 500 =503 @process_error_500; # Override the status code directly
error_page 500 = @process_error_500;    # Delegate setting the status code to the named location

https://nginx.org/en/docs/http/ngx_http_core_module.html#error_page

That's it!! Thanks!