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

Rewrite URI into request arguments #992

Open
l2dy opened this issue Nov 6, 2023 · 9 comments
Open

Rewrite URI into request arguments #992

l2dy opened this issue Nov 6, 2023 · 9 comments
Assignees

Comments

@l2dy
Copy link

l2dy commented Nov 6, 2023

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.

@hongzhidao
Copy link
Contributor

hongzhidao commented Nov 6, 2023

Hi,
Take a look at this #732 (comment).

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.

@l2dy
Copy link
Author

l2dy commented Nov 6, 2023

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.

p = nxt_cpymem(target.start, encoded_path.start, encoded_path.length);
*p++ = '?';
nxt_memcpy(p, r->args->start, r->args->length);

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

@hongzhidao
Copy link
Contributor

hongzhidao commented Nov 6, 2023

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.

@l2dy
Copy link
Author

l2dy commented Nov 6, 2023

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.

@hongzhidao
Copy link
Contributor

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.

@tippexs
Copy link
Contributor

tippexs commented Dec 6, 2023

@hongzhidao as we have worked on #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?

@hongzhidao
Copy link
Contributor

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

@tippexs
Copy link
Contributor

tippexs commented Dec 6, 2023

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

@tippexs tippexs self-assigned this Dec 6, 2023
@hongzhidao
Copy link
Contributor

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.

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

No branches or pull requests

3 participants