Fetch: separated ngx_js_http_* from ngx_js_fetch_*. #890
Conversation
hongzhidao
force-pushed
the
js-fetch-http
branch
from
7639b63 to
f9f8057
Compare
hongzhidao
force-pushed
the
js-fetch-http
branch
3 times, most recently
from
5b31cac to
499cc44
Compare
hongzhidao
force-pushed
the
js-fetch-http
branch
from
499cc44 to
091504c
Compare
hongzhidao
force-pushed
the
js-fetch-http
branch
from
091504c to
84a8f45
Compare
xeioex
left a comment
xeioex
left a comment
There was a problem hiding this comment.
Fetch: expose ngx_js_http_trim() and ngx_js_check_header_name().
can be merged into Fetch: separated ngx_js_http_* from ngx_js_fetch_*.
|
|
||
|
|
||
| ngx_int_t | ||
| ngx_njs_string(njs_vm_t *vm, njs_value_t *value, ngx_str_t *str) |
There was a problem hiding this comment.
Instead of introducing ngx_njs_string(), we should make ngx_js_string() with that prototype, which accepts ngx_str_t *str. and fix the code globally in nginx/*.
| str.start = request.method.data; | ||
| str.length = request.method.len; | ||
|
|
||
| http->header_only = njs_strstr_eq(&str, &njs_str_value("HEAD")); |
There was a problem hiding this comment.
Maybe
if (request.method.len == 4
&& ngx_strncasecmp(request.method.data, (u_char *) "HEAD", 4) == 0)
{
http->header_only = 1;
}| str.length = request->method.len; | ||
|
|
||
| for (m = &forbidden[0]; m->length != 0; m++) { | ||
| if (njs_strstr_case_eq(&request->method, m)) { |
There was a problem hiding this comment.
If we go this route with eliminating njs_str_t we should avoid here njs_strstr_case_eq() and njs_str_t .
Need to be fixed here and below.
There was a problem hiding this comment.
We could introduce in ngx_js.h something like
#define
ngx_js_strstr_case_eq(s1, s2) \
(((s1)->len == (s2)->len) \
&& (ngx_strncasecmp((s1)->start, (s2)->start, (s1)->length) == 0))
There was a problem hiding this comment.
Hi Dmitry,
I feel the string-related APIs have room for improvement, and the same for mem pool.
Ieally, there is something like ngx_js_str_t, ngx_js_mp_t, and they are related APIs around them.
It's just a draft idea.
xeioex
left a comment
xeioex
left a comment
There was a problem hiding this comment.
Fetch: expose ngx_js_http_trim() and ngx_js_check_header_name().
can be merged into Fetch: separated ngx_js_http_* from ngx_js_fetch_*.
|
Hi Dmitry, |
|
Agree. we can close this one. |
2 participants
No description provided.