unit icon indicating copy to clipboard operation
unit copied to clipboard

Rewrite URI into request arguments

Open l2dy opened this issue 2 years ago • 9 comments

I'm trying to configure Unit for Phabricator, which requires rewriting the URI into a request argument __path__ and keeping the previous request arguments.

In Nginx, this can be implemented with rewrite ^/(.*)$ /index.php?__path__=/$1 last;, but Unit does not support it.

See also #732.

l2dy avatar Nov 06 '23 12:11 l2dy

Hi, Take a look at this https://github.com/nginx/unit/issues/732#issuecomment-1621044391.

You might write the configuration like:

-- deleted --
{
  "match": {
    "uri": "~^(?<some_var>.*)$"
  },
  "action": {
     "set": {
        "args": "/index.php?__path__=/$some_var"
      }
  }
}

Unit now doesn't support custom variables from regexp with the rewrite option. We will support it in the 1.33 version. On the 1.32 currently.

hongzhidao avatar Nov 06 '23 12:11 hongzhidao

You might write the configuration like:

{
  "match": {
    "uri": "~^(?<some_var>.*)$"
  },
  "action": {
    "rewrite": "/index.php?__path__=/$some_var"
  }
}

The previous request arguments are appended after a ?. In this case, there will be two ? in the rewritten URI.

https://github.com/nginx/unit/blob/822303e23cb489efdc4fa3ca321f8468a2dd17fa/src/nxt_http_rewrite.c#L91-L93

Nginx handles this by appending a & instead of ? in ngx_http_script.c.

l2dy avatar Nov 06 '23 12:11 l2dy

Sorry, I used the wrong configuration.

https://unit.nginx.org/configuration/#uri-rewrite

It does not affect the query but changes the uri and $request_uri variables.

And we have an idea of how to change the query, but it's not clear yet. It's a topic of updating variables, like:

{
  "match": {
    "uri": "~^(?<some_var>.*)$"
  },
  "action": {
     "set": {
        "args": "/index.php?__path__=/$some_var"
      }
  }
}

Anyway, we need to support custom variables with the captured regexp names. Welcome to your suggestions on it.

hongzhidao avatar Nov 06 '23 13:11 hongzhidao

The syntax looks OK, except that I don't think ~ is needed (unless it's part of syntax to enable regex).

You will need to consider what to do with the previous request arguments (i.e. existing arguments before the rewrite). Nginx has a "putting a question mark at the end of a replacement string" way of configuring it, but it's a bit too obscure.

l2dy avatar Nov 06 '23 13:11 l2dy

Actually, in nginx, you also can set $args variables to change the request query.

You will need to consider what to do with the previous request arguments

We intend to make the usage as concise as possible, and nginx's way is more powerful with more rules. As you showed in the source code, the rewrite in nginx is very powerful with more code lines.

hongzhidao avatar Nov 06 '23 13:11 hongzhidao

@hongzhidao as we have worked on https://github.com/nginx/unit/issues/916 we made use of njs in the rewrite part with some great success in terms of regexs. For example

{
"action": {
				"rewrite": "`${uri.replace(/^(\\/[^\\/]+)?(\\/.*\\.php)/, '$2')}`",
				"share": "/sites/$host$uri",
				"fallback": {
					"pass": "applications/$host/index"
				}
			}
}

In this example we are using matching blocks of the regext to build a new URI. Coulnd't this help in this case?

tippexs avatar Dec 06 '23 14:12 tippexs

It looks like the case is related to how to change/rewrite the request query/arguments. And the rewrite option only affects the uri.

hongzhidao avatar Dec 06 '23 14:12 hongzhidao

Okay then scratch that. Let's think about enhancing the rewrite support and what release is a good fit for that.

tippexs avatar Dec 06 '23 15:12 tippexs

https://nginx.org/en/docs/http/ngx_http_rewrite_module.html

If a replacement string includes the new request arguments, the previous request arguments are appended after them. If this is undesired, putting a question mark at the end of a replacement string avoids having them appended, for example:

I would say the rule is complicated, It has advantages and disadvantages in terms of use and implementation cost.

hongzhidao avatar Dec 06 '23 15:12 hongzhidao