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

Phase Declarative Syntax for js_set #475

Open
lancedockins opened this issue Feb 20, 2022 · 2 comments
Open

Phase Declarative Syntax for js_set #475

lancedockins opened this issue Feb 20, 2022 · 2 comments

Comments

@lancedockins
Copy link

Are there any plans to modify js_set to allow specific phase declaration? Right now js_set sort of intelligently chooses its phase based on where you use the variable which is nice but if you need to use the variable in a way that has access to data that is only available at specific processing phases, there is no way to do that. For example, if we want to use a variable that can allow/deny access to a location based on the contents of the request body in a POST, this is not possible with js_set. It's only possible with js_content due to the availability of the content in the phase. If there was a way to specify that a variable must be run at an appropriate phase where the request body is available (for example), this would/should solve that.

It could be something like:
Default:
js_set module.function (picks the phase automatically like it does now)

Or declaritive:
js_set module.function access (where the phase needed is named)

Of course, if there is just some way to allow js_set variables to have access to the request body (even if that must be declarative too) that would also solve the current problem.

@xeioex
Copy link
Contributor

xeioex commented Feb 21, 2022

Hi @lancedockins,

allow specific phase declaration

please, note that nginx documentation intentionally eschews any phases mentioning (only a devguide mentions it, which is a technical document).

So, there is no such plans.

@lancedockins
Copy link
Author

@xeioex While I definitely understand your logic here, I think that this may need to be rethought. I opened a separate issue about problems accessing the body content of a request and some of Nginx's other team members have been courteous enough to keep replying to that to help deal with that.

That conversation wound up landing in a place where the Nginx phase became important. That's not because I was pushing for that. Nginx's team members brought up this problem on their own.

Variable declaration with js_set just magically decides what phase to run in. But if you explicitly need access to something like the request body in your NJS variable, you have to do all sorts of bizarre Nginx configuration workarounds to pull that off with js_set right now because the request body is only available in later phases (e.g. content phase). In the course of trying to just get access to the request body in anything other than just the js_content directive, it became clear that Nginx's phases are actually relevant to this conversation.

See this:
#474 (comment)

Basically this is my exact point with this request. Because you can't declare a phase for js_set, you can't use certain parts of the NJS API. Sometimes they will work. Sometimes they won't. It just depends on the magic of js_set behind the scenes so if js_set's magic is wrong, nothing works. So you either have to try to use clever workarounds to force NJS's phase selection magic for js_set to pick the right phase in js_set or you have to use the only js_ directives that allows you to control the phase right now.

Not only is the solution proposed in this other ticket indicative of the need to allow phase selection or at least some sort of control over whether you have access to the request body but it also sort of illustrates just how poor NJS's current API is for doing something as basic as working with the request body.

Don't get me wrong. I think NJS is awesome. But this is a pretty big hole in the basic use case for NJS. I'm not saying that a phase declaration solution is the right one. But if that doesn't exist, then something needs to be done about allowing better access to the request body. Something like this perhaps:
https://github.com/openresty/lua-nginx-module#lua_need_request_body

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

2 participants