Skip to content

ngx.location.capture - pass method names #479

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

ngx.location.capture - pass method names #479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cec65a8
ngx.location.capture - added support for passing the method optional …
Mar 24, 2015
e3f2d36
corrected style and some code according to agentzh's remarks
Apr 9, 2015
3a6a758
added some tests for the subreqest method as string feature
Apr 9, 2015
de0c6ca
Merge branch 'integration-v.10.2' into subrequest-methods
Apr 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
199 changes: 142 additions & 57 deletions src/ngx_http_lua_subrequest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@ ngx_str_t ngx_http_lua_patch_method =
ngx_str_t ngx_http_lua_trace_method =
ngx_http_lua_method_name("TRACE");

ngx_str_t * ngx_http_lua_ordered_methods[] = {
&ngx_http_lua_get_method,
&ngx_http_lua_head_method,
&ngx_http_lua_post_method,
&ngx_http_lua_put_method,
&ngx_http_lua_delete_method,
&ngx_http_lua_mkcol_method,
&ngx_http_lua_copy_method,
&ngx_http_lua_move_method,
&ngx_http_lua_options_method,
&ngx_http_lua_propfind_method,
&ngx_http_lua_proppatch_method,
&ngx_http_lua_lock_method,
&ngx_http_lua_unlock_method,
&ngx_http_lua_patch_method,
&ngx_http_lua_trace_method,
};

static ngx_str_t ngx_http_lua_content_length_header_key =
ngx_string("Content-Length");
Expand All @@ -59,7 +76,7 @@ static ngx_str_t ngx_http_lua_content_length_header_key =
static ngx_int_t ngx_http_lua_set_content_length_header(ngx_http_request_t *r,
off_t len);
static ngx_int_t ngx_http_lua_adjust_subrequest(ngx_http_request_t *sr,
ngx_uint_t method, int forward_body,
ngx_uint_t method, ngx_str_t *method_name, int forward_body,
ngx_http_request_body_t *body, unsigned vars_action,
ngx_array_t *extra_vars);
static int ngx_http_lua_ngx_location_capture(lua_State *L);
Expand Down Expand Up @@ -139,6 +156,11 @@ ngx_http_lua_ngx_location_capture_multi(lua_State *L)
unsigned vars_action;
ngx_uint_t nsubreqs;
ngx_uint_t index;
ngx_uint_t method_index;
ngx_uint_t methods_number;
ngx_str_t *ngx_method_name = NULL;
u_char *lua_method_name;
size_t method_name_length;
size_t sr_statuses_len;
size_t sr_headers_len;
size_t sr_bodies_len;
Expand Down Expand Up @@ -383,15 +405,68 @@ ngx_http_lua_ngx_location_capture_multi(lua_State *L)

type = lua_type(L, -1);

if (type == LUA_TNIL) {
switch (type) {
case LUA_TNIL:
method = NGX_HTTP_GET;
break;

} else {
if (type != LUA_TNUMBER) {
case LUA_TNUMBER:
method = (ngx_uint_t) lua_tonumber(L, -1);
break;

case LUA_TSTRING:
lua_method_name = (u_char *) lua_tolstring(L, -1,
&method_name_length);
if (method_name_length == 0) {
return luaL_error(L, "Bad http request method");
}

methods_number = sizeof(ngx_http_lua_ordered_methods) /
sizeof(ngx_http_lua_ordered_methods[0]);

for (method_index = 0; method_index < methods_number;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

@detailyang detailyang Nov 22, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@detailyang duplicating this big snippet of code is rather horrifying :) Better be in a separate C function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@detailyang Sounds good.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agentzh i will try create a new PR based on this PR to optimizations this code block:)

method_index++)
{
if (ngx_strncasecmp(
ngx_http_lua_ordered_methods[method_index]->data,
lua_method_name,
method_name_length)
== 0) {
break;
}
}

method = (ngx_uint_t) lua_tonumber(L, -1);
if (method_index == methods_number) {
/* unknown method */
method = NGX_HTTP_UNKNOWN;
ngx_method_name = ngx_palloc(r->pool,
sizeof(ngx_str_t));
if (ngx_method_name == NULL) {
return luaL_error(L, "out of memory");
}

ngx_method_name->data = ngx_palloc(r->pool,
method_name_length);
if (ngx_method_name->data == NULL) {
return luaL_error(L, "out of memory");
}

ngx_memcpy(ngx_method_name->data,
lua_method_name,
method_name_length);
ngx_method_name->data[method_name_length] = ' ';
ngx_method_name->len = method_name_length;

} else {
/* the method is a bit field, the first value is
of NGX_HTTP_GET */
method = NGX_HTTP_GET << method_index;
ngx_method_name = NULL;
}
break;

default:
return luaL_error(L, "Bad http request method");
}

lua_pop(L, 1);
Expand Down Expand Up @@ -582,7 +657,8 @@ ngx_http_lua_ngx_location_capture_multi(lua_State *L)

ngx_http_set_ctx(sr, sr_ctx, ngx_http_lua_module);

rc = ngx_http_lua_adjust_subrequest(sr, method, always_forward_body,
rc = ngx_http_lua_adjust_subrequest(sr, method, ngx_method_name,
always_forward_body,
body, vars_action, extra_vars);

if (rc != NGX_OK) {
Expand Down Expand Up @@ -618,8 +694,9 @@ ngx_http_lua_ngx_location_capture_multi(lua_State *L)

static ngx_int_t
ngx_http_lua_adjust_subrequest(ngx_http_request_t *sr, ngx_uint_t method,
int always_forward_body, ngx_http_request_body_t *body,
unsigned vars_action, ngx_array_t *extra_vars)
ngx_str_t *method_name, int always_forward_body,
ngx_http_request_body_t *body, unsigned vars_action,
ngx_array_t *extra_vars)
{
ngx_http_request_t *r;
ngx_int_t rc;
Expand Down Expand Up @@ -676,71 +753,79 @@ ngx_http_lua_adjust_subrequest(ngx_http_request_t *sr, ngx_uint_t method,
sr->method = method;

switch (method) {
case NGX_HTTP_GET:
sr->method_name = ngx_http_lua_get_method;
break;
case NGX_HTTP_UNKNOWN:
if (method_name == NULL) {
return NGX_ERROR;
}

case NGX_HTTP_POST:
sr->method_name = ngx_http_lua_post_method;
break;
sr->method_name = *method_name;
break;

case NGX_HTTP_GET:
sr->method_name = ngx_http_lua_get_method;
break;

case NGX_HTTP_PUT:
sr->method_name = ngx_http_lua_put_method;
break;
case NGX_HTTP_POST:
sr->method_name = ngx_http_lua_post_method;
break;

case NGX_HTTP_HEAD:
sr->method_name = ngx_http_lua_head_method;
break;
case NGX_HTTP_PUT:
sr->method_name = ngx_http_lua_put_method;
break;

case NGX_HTTP_DELETE:
sr->method_name = ngx_http_lua_delete_method;
break;
case NGX_HTTP_HEAD:
sr->method_name = ngx_http_lua_head_method;
break;

case NGX_HTTP_OPTIONS:
sr->method_name = ngx_http_lua_options_method;
break;
case NGX_HTTP_DELETE:
sr->method_name = ngx_http_lua_delete_method;
break;

case NGX_HTTP_MKCOL:
sr->method_name = ngx_http_lua_mkcol_method;
break;
case NGX_HTTP_OPTIONS:
sr->method_name = ngx_http_lua_options_method;
break;

case NGX_HTTP_COPY:
sr->method_name = ngx_http_lua_copy_method;
break;
case NGX_HTTP_MKCOL:
sr->method_name = ngx_http_lua_mkcol_method;
break;

case NGX_HTTP_MOVE:
sr->method_name = ngx_http_lua_move_method;
break;
case NGX_HTTP_COPY:
sr->method_name = ngx_http_lua_copy_method;
break;

case NGX_HTTP_PROPFIND:
sr->method_name = ngx_http_lua_propfind_method;
break;
case NGX_HTTP_MOVE:
sr->method_name = ngx_http_lua_move_method;
break;

case NGX_HTTP_PROPPATCH:
sr->method_name = ngx_http_lua_proppatch_method;
break;
case NGX_HTTP_PROPFIND:
sr->method_name = ngx_http_lua_propfind_method;
break;

case NGX_HTTP_LOCK:
sr->method_name = ngx_http_lua_lock_method;
break;
case NGX_HTTP_PROPPATCH:
sr->method_name = ngx_http_lua_proppatch_method;
break;

case NGX_HTTP_UNLOCK:
sr->method_name = ngx_http_lua_unlock_method;
break;
case NGX_HTTP_LOCK:
sr->method_name = ngx_http_lua_lock_method;
break;

case NGX_HTTP_PATCH:
sr->method_name = ngx_http_lua_patch_method;
break;
case NGX_HTTP_UNLOCK:
sr->method_name = ngx_http_lua_unlock_method;
break;

case NGX_HTTP_TRACE:
sr->method_name = ngx_http_lua_trace_method;
break;
case NGX_HTTP_PATCH:
sr->method_name = ngx_http_lua_patch_method;
break;

default:
ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
"unsupported HTTP method: %u", (unsigned) method);
case NGX_HTTP_TRACE:
sr->method_name = ngx_http_lua_trace_method;
break;

return NGX_ERROR;
default:
ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
"unsupported HTTP method: %u", (unsigned) method);

return NGX_ERROR;
}

if (!(vars_action & NGX_HTTP_LUA_SHARE_ALL_VARS)) {
Expand Down
Loading