nginx/njs

NJS call wrong function.

Closed this issue · 7 comments

mingkyme commented

When use same variable name, it called wrong function.

Here is an example.

server {
  server_name a;
  listen 80;
  js_set $value main.A;
  add_header X-A-Value $value;
  location / {
    return 200;
  }
}

server {
  server_name b;
  listen 80;
  js_set $value main.B;
  add_header X-B-Value $value;
  location / {
    return 200;
  }
}

function A(r){
  return 1;
}

function B(r){
  return 2;
}
export default { A,B }
curl -I 0 -H "Host: a"
HTTP/1.1 200 OK
Server: nginx/1.24.0
Date: Wed, 03 Apr 2024 04:14:50 GMT
Content-Type: application/octet-stream
Content-Length: 0
Connection: keep-alive
X-A-Value: 2
xeioex commented

Hi @mingkyme,

js_set variables (as all nginx variables) are global, so basically $value is declared twice and the second declaration shadows the first one. js_set is different from ordinary set because js_set does not init the variable.

I am going to add some checks though to detect situations like this.

  • 👍1
zsteinkamp commented

Would be nice to have at least server{} block scoped vars. It's surprising that they're global. If this is impossible, then I guess the next best thing is to write a warning into the error log.

xeioex commented

@arut Roman, do you have an opinion about server{} scope nginx variables?

arut commented

The variables are global by design. It's hardly possible to change it without significant refactoring of nginx. To avoid such misconfigurations, it should be advised to place js_set at http{] level. These variables hold different values, then why do they have the same name?

NetForce1 commented

@arut I don't know about @mingkyme , but for me the reason for wanting server-scoped variables is to have generic NJS functions that need different configuration per server. Like in njs-acme: nginx/njs-acme#57.

Without server-scoped variable, the only proper way to do that would be to create a separate function per server that calls the generic function with the server-specific config?

zsteinkamp commented

Heya @NetForce1 -- A colleague suggested I explore using a map keyed by $host. I will hopefully have time to dig into this before the end of the week and will report back here how it might work with njs-acme.

NetForce1 commented
  • 👍1