Thanks a lot for reply. I will try make the adjustments accordingly.<br><br>> Any reason to use value here? Looks like cut-n-paste leftover<br>> from gzip_disable.<br><br>Yes, I did copy those from gzip_disable. be frankly, I'm still not very clear on the correct ways to use ngx_buf , ngx_string , ngx_regex . Also not sure where to look except code sample from other module :| . <br>
<br>For example. I'm thinking to detect if the xml do fit for xslt transform before put it though, which mean the xml should have line like <?xml-stylesheet type="text/xsl" href="xxxxx"/> . I guess I should put it at beginning of ngx_http_xslt_body_filter. and check if I can find "xml-stylesheet" in (ngx_chain_t *)in->buf. But not sure what's the correct way to handle ngx_buf .<br>
<br>I'll subscribe nginx-devel now.<br><br><br><div class="gmail_quote">On Wed, Jan 27, 2010 at 8:49 PM, Maxim Dounin <span dir="ltr"><<a href="mailto:mdounin@mdounin.ru">mdounin@mdounin.ru</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">Hello!<br>
<br>
Just a note: it's probably better idea to post patches to<br>
nginx-devel@. Cc'd.<br>
<div class="im"><br>
On Wed, Jan 27, 2010 at 06:04:16PM +0800, tOmasEn wrote:<br>
<br>
> Maybe not many people using xslt as main web layout. but I do.<br>
><br>
> This patch is similar to gzip_disable. xslt_enable only enable xslt<br>
> transform for certain user agent.<br>
><br>
> e.g.<br>
> xslt_enable "Googlebot"<br>
> will only enable xslt transform output for google's crawler.<br>
<br>
</div>While I tend to think this is good feature (though not sure), I<br>
also think that it probably needs some better name. Something<br>
more self-explaining.<br>
<br>
Also it's probably better to make xslt controllable via variable<br>
instead. This will handle you "disable via special header" case<br>
as well.<br>
<div class="im"><br>
> And, I'm also planing some other improve on xslt module. Like: when xslt<br>
> transform failed, output original xml file instead of 500 server error; Skip<br>
> xslt transform if client send specified HTTP header like "No-XSLT", etc.<br>
> Should I talk to somebody and discuss the architect?<br>
><br>
> --<br>
> Tomasen<br>
> <a href="http://twitter.com/ShooterPlayer" target="_blank">http://twitter.com/ShooterPlayer</a><br>
> <a href="http://t.sina.com.cn/Tomasen" target="_blank">http://t.sina.com.cn/Tomasen</a><br>
<br>
</div>> *** ../nginx-0.8.31/src/http/modules/ngx_http_xslt_filter_module.c 2010-01-27 17:23:37.000000000 +0800<br>
> --- nginx-0.8.31/src/http/modules/ngx_http_xslt_filter_module.c 2009-11-30 21:15:10.000000000 +0800<br>
> ***************<br>
> *** 48,54 ****<br>
<br>
Please use unified diff patches, and not reversed ones.<br>
<br>
> ngx_array_t sheets; /* ngx_http_xslt_sheet_t */<br>
> ngx_hash_t types;<br>
> ngx_array_t *types_keys;<br>
> - ngx_array_t *xslt_enable; /* xslt_enable */<br>
<br>
Comment is pointless and wrongly placed. Just remove it.<br>
<br>
> } ngx_http_xslt_filter_loc_conf_t;<br>
><br>
><br>
> --- 48,53 ----<br>
> ***************<br>
> *** 135,143 ****<br>
> void *child);<br>
> static ngx_int_t ngx_http_xslt_filter_init(ngx_conf_t *cf);<br>
> static void ngx_http_xslt_filter_exit(ngx_cycle_t *cycle);<br>
> !<br>
> ! static char *ngx_http_xslt_enable(ngx_conf_t *cf, ngx_command_t *cmd,<br>
> ! void *conf);<br>
<br>
1. Please follow nginx style.<br>
<br>
2. Please move this to other config parsing functions.<br>
<br>
><br>
> ngx_str_t ngx_http_xslt_default_types[] = {<br>
> ngx_string("text/xml"),<br>
> --- 134,140 ----<br>
> void *child);<br>
> static ngx_int_t ngx_http_xslt_filter_init(ngx_conf_t *cf);<br>
> static void ngx_http_xslt_filter_exit(ngx_cycle_t *cycle);<br>
> !<br>
><br>
> ngx_str_t ngx_http_xslt_default_types[] = {<br>
> ngx_string("text/xml"),<br>
> ***************<br>
> *** 168,183 ****<br>
> offsetof(ngx_http_xslt_filter_loc_conf_t, types_keys),<br>
> &ngx_http_xslt_default_types[0] },<br>
><br>
> ! { ngx_string("xslt_enable"),<br>
> ! NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_1MORE,<br>
> ! ngx_http_xslt_enable,<br>
> ! NGX_HTTP_LOC_CONF_OFFSET,<br>
> ! offsetof(ngx_http_xslt_filter_loc_conf_t, xslt_enable),<br>
> ! NULL },<br>
> !<br>
> ! ngx_null_command<br>
> };<br>
><br>
> static ngx_http_module_t ngx_http_xslt_filter_module_ctx = {<br>
> NULL, /* preconfiguration */<br>
> ngx_http_xslt_filter_init, /* postconfiguration */<br>
> --- 165,174 ----<br>
> offsetof(ngx_http_xslt_filter_loc_conf_t, types_keys),<br>
> &ngx_http_xslt_default_types[0] },<br>
><br>
> ! ngx_null_command<br>
> };<br>
><br>
> +<br>
<br>
Unrelated (and wrong) whitespace change here.<br>
<br>
> static ngx_http_module_t ngx_http_xslt_filter_module_ctx = {<br>
> NULL, /* preconfiguration */<br>
> ngx_http_xslt_filter_init, /* postconfiguration */<br>
> ***************<br>
> *** 212,289 ****<br>
> static ngx_http_output_header_filter_pt ngx_http_next_header_filter;<br>
> static ngx_http_output_body_filter_pt ngx_http_next_body_filter;<br>
><br>
> -<br>
> - static char *<br>
> - ngx_http_xslt_enable(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)<br>
> - {<br>
<br>
Please move config parsing function to other config parsing<br>
functions.<br>
<br>
> - ngx_http_xslt_filter_loc_conf_t *clcf = conf;<br>
> -<br>
> - #if (NGX_PCRE)<br>
> - ngx_str_t *value;<br>
> - ngx_uint_t i;<br>
> - ngx_regex_elt_t *re;<br>
> - ngx_regex_compile_t rc;<br>
> - u_char errstr[NGX_MAX_CONF_ERRSTR];<br>
> -<br>
> - if (clcf->xslt_enable == NULL) {<br>
> - clcf->xslt_enable = ngx_array_create(cf->pool, 2,<br>
> - sizeof(ngx_regex_elt_t));<br>
> - if (clcf->xslt_enable == NULL) {<br>
> - return NGX_CONF_ERROR;<br>
> - }<br>
<br>
No tabs, please.<br>
<br>
> - }else{<br>
> - //xslt_enable_is_not_required<br>
> - return NGX_CONF_OK;<br>
<br>
1. No C++ comments, please.<br>
<br>
2. You should either return error here (something like return "is<br>
duplicate") or append to list as gzip_disable do. Blindly<br>
ignoring configuration directive isn't what nginx generally do.<br>
<br>
> - }<br>
> -<br>
> - value = cf->args->elts;<br>
> -<br>
> - ngx_memzero(&rc, sizeof(ngx_regex_compile_t));<br>
> -<br>
> - rc.pool = cf->pool;<br>
> - rc.err.len = NGX_MAX_CONF_ERRSTR;<br>
> - rc.err.data = errstr;<br>
> -<br>
> - for (i = 1; i < cf->args->nelts; i++) {<br>
> -<br>
> - re = ngx_array_push(clcf->xslt_enable);<br>
> - if (re == NULL) {<br>
> - return NGX_CONF_ERROR;<br>
> - }<br>
> -<br>
> - rc.pattern = value[1];<br>
> - rc.options = NGX_REGEX_CASELESS;<br>
> -<br>
> - if (ngx_regex_compile(&rc) != NGX_OK) {<br>
> - ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "%V", &rc.err);<br>
> - return NGX_CONF_ERROR;<br>
> - }<br>
> -<br>
> - re->regex = rc.regex;<br>
> - re->name = value[i].data;<br>
> -<br>
> - }<br>
> -<br>
> - return NGX_CONF_OK;<br>
> -<br>
> - #else<br>
> - ngx_str_t *value;<br>
> -<br>
> - value = cf->args->elts;<br>
<br>
Any reason to use value here? Looks like cut-n-paste leftover<br>
from gzip_disable.<br>
<br>
> -<br>
> - ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,<br>
> - "without PCRE library \"xslt_enable\" won't work ");<br>
> -<br>
> - return NGX_CONF_ERROR;<br>
> - #endif<br>
> - }<br>
><br>
> static ngx_int_t<br>
> ngx_http_xslt_header_filter(ngx_http_request_t *r)<br>
> {<br>
> ngx_http_xslt_filter_ctx_t *ctx;<br>
> ngx_http_xslt_filter_loc_conf_t *conf;<br>
> !<br>
> ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,<br>
> "xslt filter header");<br>
><br>
> --- 203,215 ----<br>
> static ngx_http_output_header_filter_pt ngx_http_next_header_filter;<br>
> static ngx_http_output_body_filter_pt ngx_http_next_body_filter;<br>
><br>
><br>
> static ngx_int_t<br>
> ngx_http_xslt_header_filter(ngx_http_request_t *r)<br>
> {<br>
> ngx_http_xslt_filter_ctx_t *ctx;<br>
> ngx_http_xslt_filter_loc_conf_t *conf;<br>
> !<br>
> ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,<br>
> "xslt filter header");<br>
><br>
> ***************<br>
> *** 299,318 ****<br>
> return ngx_http_next_header_filter(r);<br>
> }<br>
><br>
> - #if (NGX_PCRE)<br>
> -<br>
> - if (conf->xslt_enable && r->headers_in.user_agent) {<br>
> -<br>
> - if (ngx_regex_exec_array(conf->xslt_enable,<br>
> - &r->headers_in.user_agent->value,<br>
> - r->connection->log)<br>
> - == NGX_DECLINED)<br>
> - {<br>
> - return ngx_http_next_header_filter(r);<br>
> - }<br>
> - }<br>
> -<br>
> - #endif<br>
> ctx = ngx_http_get_module_ctx(r, ngx_http_xslt_filter_module);<br>
><br>
> if (ctx) {<br>
> --- 225,230 ----<br>
<font color="#888888"><br>
Maxim Dounin<br>
</font><div><div></div><div class="h5"><br>
_______________________________________________<br>
nginx mailing list<br>
<a href="mailto:nginx@nginx.org">nginx@nginx.org</a><br>
<a href="http://nginx.org/mailman/listinfo/nginx" target="_blank">http://nginx.org/mailman/listinfo/nginx</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>Tomasen<br><a href="http://twitter.com/ShooterPlayer">http://twitter.com/ShooterPlayer</a><br><a href="http://t.sina.com.cn/Tomasen">http://t.sina.com.cn/Tomasen</a><br>