Age | Commit message (Collapse) | Author | Files | Lines |
|
Previously, the REQUEST_URI within Unit could be modified,
for example, during uri rewriting. We decide to make $request_uri
immutable and pass constant REQUEST_URI to applications.
Based on the new requirement, we remove `r->target` rewriting
in the rewrite module.
Closes: https://github.com/nginx/unit/issues/916
Reviewed-by: Andrew Clayton <a.clayton@nginx.com>
Signed-off-by: Zhidao HONG <z.hong@f5.com>
|
|
Previously, proxy request was constructed based on the `r->target`
field. However, r->target will remain unchanged in the future,
even in cases of URL rewriting because of the requirement change
for $request_uri that will be changed to constant.
To accommodate this, the r->target should be designed to be constant,
but Unit needs to pass a changeable URL to the upstream server.
Based on the above, the proxy module can't depend on r->target.
|
|
On Github, @rlandgrebe reported an issue when trying to rewrite URLs
that contained query strings.
With the PHP language module we were in fact segfaulting (SIGSEGV) in
libphp
[93960.462952] unitd[20940]: segfault at 7f307cef6476 ip 00007f2f81a94577 sp 00007fff28a777d0 error 4 in libphp-8.2.so[7f2f818df000+2fd000] likely on CPU 0 (core 0, socket 0)
#0 0x00007f2abd494577 in php_default_treat_data (arg=1, str=0x0,
destArray=<optimized out>)
at /usr/src/debug/php-8.2.10-1.fc38.x86_64/main/php_variables.c:488
488 if (c_var && *c_var) {
(gdb) p c_var
$1 = 0x7f2bb8880676 <error: Cannot access memory at address 0x7f2bb8880676>
This was when trying to get the query string which somehow is pointing
off into the woods.
This gdb debug session when doing rewrite basically shows the core of
the issue
(gdb) x /64bs req->fields
...
0x7f7eaaaa8090: "GET"
0x7f7eaaaa8094: "HTTP/1.1"
0x7f7eaaaa809d: "::1"
0x7f7eaaaa80a1: "::1"
0x7f7eaaaa80a5: "8080"
0x7f7eaaaa80aa: "localhost"
0x7f7eaaaa80b4: "/test?q=a"
0x7f7eaaaa80be: "/test"
...
(gdb) p target_pos
$4 = (void *) 0x7f7eaaaa80b4
(gdb) p query_pos
$6 = (void *) 0x7f7eaaaa6af6
(gdb) p r->args->start
$8 = (u_char *) 0x7f7ea4002b02 "q=a HTTP/1.1\r\nHost: localhost:8080\r\nUser-Agent: curl/8.0.1\r\nAccept: */*\r\n\r\n"
(gdb) p r->target.start
$9 = (u_char *) 0x7f7ea40040c0 "/test?q=a"
That last address, 0x7f7ea40040c0, looks out of wack, it should be
smaller than r->args->start.
That results in a calculation in nxt_router_prepare_msg()
if (r->args->start != NULL) {
query_pos = nxt_pointer_to(target_pos,
r->args->start - r->target.start);
nxt_unit_sptr_set(&req->query, query_pos);
} else {
that goes negative that then is stored in req->query.offset which is a
uint32_t and so wraps backwards from UINT_MAX to give us an offset of a
little under 4GiB, hence the above invalid memory access.
All this happens due to in nxt_http_rewrite() if we have a URL with a
query string, we create a new memory allocation to store the transformed
URL and query string.
We set r->target to point to this new allocation, but we also need to
point r->args->start to the start of the query string in this new
allocation.
Reported-by: René Landgrebe <https://github.com/rlandgrebe>
Tested-by: René Landgrebe <https://github.com/rlandgrebe>
Tested-by: Liam Crilly <liam.crilly@nginx.com>
Fixes: 14d6d97b ("HTTP: added basic URI rewrite.")
Closes: <https://github.com/nginx/unit/issues/964>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
|
|
No functional changes.
|
|
Signed-off-by: Alejandro Colomar <alx@nginx.com>
|
|
This commit introduced the basic URI rewrite. It allows users to change request URI. Note the "rewrite" option ignores the contained query if any and the query from the request is preserverd.
An example:
"routes": [
{
"match": {
"uri": "/v1/test"
},
"action": {
"return": 200
}
},
{
"action": {
"rewrite": "/v1$uri",
"pass": "routes"
}
}
]
Reviewed-by: Alejandro Colomar <alx@nginx.com>
|