Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NJS call wrong function. #700

Closed
mingkyme opened this issue Apr 3, 2024 · 7 comments
Closed

NJS call wrong function. #700

mingkyme opened this issue Apr 3, 2024 · 7 comments

Comments

@mingkyme
Copy link

mingkyme commented Apr 3, 2024

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
Copy link
Contributor

xeioex commented Apr 3, 2024

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.

@zsteinkamp
Copy link

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
Copy link
Contributor

xeioex commented Apr 3, 2024

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

@arut
Copy link
Contributor

arut commented Apr 8, 2024

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
Copy link

@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
Copy link

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
Copy link

NetForce1 commented Apr 9, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants