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