unit
unit copied to clipboard
HTTP: improved request uri.
- Another implementation of proxy request creation.
- Made the
$request_uri
constant. - Passed constant
REQUEST_URI
to applications.
Need to separate the PR after tests pass.
Here's a manual test.
# conf.json
{
"listeners": {
"*:8080": {
"pass": "routes/a"
},
"*:8081": {
"pass": "routes/b"
}
},
"routes": {
"a": [
{
"action": {
"rewrite": "/foo.php",
"proxy": "http://127.0.0.1:8081"
}
}
],
"b": [
{
"action": {
"rewrite": "/index.php",
"pass": "applications/app"
}
}
]
},
"applications": {
"app": {
"type": "php",
"root": "/www",
"index": "index.php"
}
}
}
# /www/index.php
<?php
phpinfo();
> curl http://127.1:8080
$_SERVER['REQUEST_URI'] => /foo.php
Hi @andrey-zelenkov, Since this is a change, we need to add more tests to it.
- add rewrite on "proxy" which is similar to "php application rewrite". It doesn't depend on the PR.
- change the tests for
$request_uri
if possible. - change the tests for "applications" if possible. Thanks.
Hmm, I thought the idea was that REQUEST_URI
doesn't get munged?
> curl http://127.1:8080 $_SERVER['REQUEST_URI'] => /foo.php
In which case I would have expected REQUEST_URI
would be set to /
in the above instance.
Hmm, I thought the idea was that
REQUEST_URI
doesn't get munged?> curl http://127.1:8080 $_SERVER['REQUEST_URI'] => /foo.php
In which case I would have expected
REQUEST_URI
would be set to/
in the above instance.
Here are a few namings:
-
REQEUST_URI
is for applications, it must be unchanged. -
$request_uri
corresponding internalr->target
, both of them are unchanged.
So your understanding is correct. For the above configuration, there are two requests. The first one is the client request and is accepted by 8080.
"action": {
"rewrite": "/foo.php",
"proxy": "http://127.0.0.1:8081"
}
The request_uri
is unchanged. Then Units generates the second request with a rewritten uri to pass to 8081.
Now the second request is /foo.php
, and it will be accepted by 8081.
{
"action": {
"rewrite": "/index.php",
"pass": "applications/app"
}
}
Hi, I created a new PR https://github.com/nginx/unit/pull/1214 prepared for this patch, and it will simplify this PR a lot. I'll rework this PR after https://github.com/nginx/unit/pull/1214 and https://github.com/nginx/unit/pull/1211 are shipped.
@andrey-zelenkov @hongzhidao: I want to double check the failing tests: https://github.com/nginx/unit/actions/runs/8718395388/job/23915553886?pr=1162#step:41:1060
I tested manually, and given the configuration in the test and the request, the log line ends up being
::1 /bar /blah%2fblah?a=b
192.168.65.1 /foo /blah%2fblah?a=b
This is correct given the request URI. I think those tests should be changed given the outcome of the code they're testing has been changed in this PR.
I'm working on a PR to this branch at https://github.com/hongzhidao/unit/pull/1, but wanted to run this by you as you're most familiar with the tests 🙂
I'll keep working on that PR as I fix the tests.