ngx.location.capture - pass method names #479
Conversation
…parameter as a string. If the method string isn't a known Nginx method, it is still passed (and Nginx considers it as unknown).
src/ngx_http_lua_subrequest.c
Outdated
|
|
||
| methods_number = sizeof(ngx_http_lua_ordered_methods) / | ||
| sizeof(ngx_http_lua_ordered_methods[0]); | ||
| for (method_index = 0; method_index < methods_number; |
There was a problem hiding this comment.
Style: need a blank line before for loop.
|
And again, we need some test cases for this new feature in our existing test suite :) Thank you for your hard work! |
|
@agentzh , made the requested corrections and added test cases |
| methods_number = sizeof(ngx_http_lua_ordered_methods) / | ||
| sizeof(ngx_http_lua_ordered_methods[0]); | ||
|
|
||
| for (method_index = 0; method_index < methods_number; |
There was a problem hiding this comment.
Hmm, I'm afraid I don't quite like this O(n) operation on such a hot code path. Maybe introduce a hash table here?
There was a problem hiding this comment.
@agentzh maybe we can do this as the following, i copy this from ngx_http_parse.c:
switch (p - m) {
case 3:
if (ngx_str3_cmp(m, 'G', 'E', 'T', ' ')) {
r->method = NGX_HTTP_GET;
break;
}
if (ngx_str3_cmp(m, 'P', 'U', 'T', ' ')) {
r->method = NGX_HTTP_PUT;
break;
}
break;
case 4:
if (m[1] == 'O') {
if (ngx_str3Ocmp(m, 'P', 'O', 'S', 'T')) {
r->method = NGX_HTTP_POST;
break;
}
if (ngx_str3Ocmp(m, 'C', 'O', 'P', 'Y')) {
r->method = NGX_HTTP_COPY;
break;
}
if (ngx_str3Ocmp(m, 'M', 'O', 'V', 'E')) {
r->method = NGX_HTTP_MOVE;
break;
}
if (ngx_str3Ocmp(m, 'L', 'O', 'C', 'K')) {
r->method = NGX_HTTP_LOCK;
break;
}
} else {
if (ngx_str4cmp(m, 'H', 'E', 'A', 'D')) {
r->method = NGX_HTTP_HEAD;
break;
}
}
break;
case 5:
if (ngx_str5cmp(m, 'M', 'K', 'C', 'O', 'L')) {
r->method = NGX_HTTP_MKCOL;
break;
}
if (ngx_str5cmp(m, 'P', 'A', 'T', 'C', 'H')) {
r->method = NGX_HTTP_PATCH;
break;
}
if (ngx_str5cmp(m, 'T', 'R', 'A', 'C', 'E')) {
r->method = NGX_HTTP_TRACE;
break;
}
break;
case 6:
if (ngx_str6cmp(m, 'D', 'E', 'L', 'E', 'T', 'E')) {
r->method = NGX_HTTP_DELETE;
break;
}
if (ngx_str6cmp(m, 'U', 'N', 'L', 'O', 'C', 'K')) {
r->method = NGX_HTTP_UNLOCK;
break;
}
break;
case 7:
if (ngx_str7_cmp(m, 'O', 'P', 'T', 'I', 'O', 'N', 'S', ' '))
{
r->method = NGX_HTTP_OPTIONS;
}
break;
case 8:
if (ngx_str8cmp(m, 'P', 'R', 'O', 'P', 'F', 'I', 'N', 'D'))
{
r->method = NGX_HTTP_PROPFIND;
}
break;
case 9:
if (ngx_str9cmp(m,
'P', 'R', 'O', 'P', 'P', 'A', 'T', 'C', 'H'))
{
r->method = NGX_HTTP_PROPPATCH;
}
break;
}
what do you think of it 😄
There was a problem hiding this comment.
@detailyang duplicating this big snippet of code is rather horrifying :) Better be in a separate C function?
There was a problem hiding this comment.
@agentzh yep, i cannot agree more:). we can put this snippet of code as the helper function to ngx_http_lua_util.c. is it okay?
There was a problem hiding this comment.
@agentzh i will try create a new PR based on this PR to optimizations this code block:)
e7ac10c to
cfd4f90
Compare
|
Is this issue still getting merge? |
|
@mingfang Yes. Though needs some cleanup and optimizations. |
Conflicts: t/020-subrequest.t
|
@agentzh Is this going to get merged? I would also find this feature really useful. |
|
This PR needs some coding style fixes and should be rebased to the latest master. It's currently having conflicts with the latest master. |
f924579 to
fef2581
Compare
|
This pull request is now in conflict :( |
|
Really need this feature |
Would you please rebase the code to the latest master? |
|
This pull request is now in conflict :( |
|
This pull request is now in conflict :( |
|
This pull request is now in conflict :( |
|
This pull request is now in conflict :( |
|
This pull request is now in conflict :( |
Added the option to pass the method of the subrequests to ngx.location.capture as a string in addition to the integer based value.
This is an update (and a cleaner diff) of pull request #289