<div dir="ltr">Maxim,<div><br></div><div>I'm doing line by line documentation in my module. I hope by doing this I will get a good understanding of what is going on. If allowed I intend to commit this as a resource for others also intending to learn the body filter system (I will however check with this mailing list to see if anyone has an issue with my descriptions).</div><div><br></div><div>While doing this I noticed that ctx->flush does not appear to ever be true in your gunzip module. Am I missing something here?</div><div><br></div><div>Regards,</div><div>Mathew</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 16 Apr 2020 at 10:49, Mathew Heard <<a href="mailto:me@mheard.com">me@mheard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Maxim,<br><br>Which clause of the 2 clause BSD license am I violating? It's not my intention to violate any. If need be I will remove this project from distribution and take it closed source. It would be a shame but if it needs to be done...<br><br>>> Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:<div><br></div><div>Agreed, distribution with modification.<br><br>>> Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.<br><br><a href="https://github.com/splitice/ngx_brunzip_module/blob/master/LICENSE.md" target="_blank">https://github.com/splitice/ngx_brunzip_module/blob/master/LICENSE.md</a>  <br><br>>> <span style="color:rgb(0,0,0);white-space:pre-wrap"> Redistributions in binary form must reproduce the above copyright </span><span style="color:rgb(0,0,0);white-space:pre-wrap">notice, this list of conditions and the following disclaimer in the </span><span style="color:rgb(0,0,0);white-space:pre-wrap">documentation and/or other materials provided with the distribution.</span><span style="color:rgb(0,0,0);white-space:pre-wrap"><br></span><br>No binary distribution is performed. </div><div><br></div><div>>> [etcetera]</div><div><br></div><div>There is no express intent to apply any warranty to you or Nginx inc.<br></div><div><br></div><div>I'll add some copyright lines at the top of that file as those should probably be there, I'm not sure they have any legal implication (copyright is inheint) but better to give credit for the functions used as a reference.</div><div><br></div><div>As for the assistance I am digesting that now. I see what you mean though regarding available_out == 0, that's indeed a problem. I'll go through the path for my < 64 patch too, that was not the intent.</div><div><br></div><div>I'm aware of that documentation, I guess I was hoping that there was more.</div><div><br></div><div>Regards,<br>Mathew</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 16 Apr 2020 at 03:12, Maxim Dounin <<a href="mailto:mdounin@mdounin.ru" target="_blank">mdounin@mdounin.ru</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello!<br>
<br>
On Wed, Apr 15, 2020 at 10:21:09AM +1000, Mathew Heard wrote:<br>
<br>
> Hi all,<br>
> <br>
> I'm the maintainer of an open source module ngx_brunzip_module (<br>
> <a href="https://github.com/splitice/ngx_brunzip_module/" rel="noreferrer" target="_blank">https://github.com/splitice/ngx_brunzip_module/</a><br>
> <<a href="https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c" rel="noreferrer" target="_blank">https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c</a>>).<br>
> Effectively the same as the gunzip module (and based off that source) but<br>
> with Brotli.<br>
<br>
If it's based on the gunzip module code, you are violating <br>
copyrights on the code, including mine.  Please fix.<br>
<br>
> I've been scratching my head for 2 days regarding some high CPU usage<br>
> within the chain code. It appears that some spinning is possible. I must<br>
> admit I only have a basic understanding of the filter chain in nginx (still<br>
> gaining experience).<br>
> <br>
> 1. I was wondering if someone could take a look at the code and give me<br>
> some pointers?<br>
<br>
Likely unrelated, but "ctx->input" and "ctx->output" are <br>
meaningless and never used.<br>
<br>
Likely unrealted, but "ctx->flush = FLUSH_NOFLUSH" at<br>
<a href="https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L393" rel="noreferrer" target="_blank">https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L393</a> <br>
is meaningless.<br>
<br>
> 2. Also I've added some code to prevent further filling of mostly full<br>
> buffers (as it appears brotli is quite expensive to start) at<br>
> <a href="https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L408" rel="noreferrer" target="_blank">https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L408</a><br>
> is<br>
> this valid? How does nginx determine when backpressure from full output<br>
> chains is relieved? Is there any in-depth documentation of the filter chain<br>
> architecture?<br>
<br>
No, it's not valid, and your code will throw away such mostly <br>
filled output buffers without linking them to the output chain as <br>
normally happens in ngx_http_brunzip_filter_inflate() at<br>
<br>
<a href="https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L485" rel="noreferrer" target="_blank">https://github.com/splitice/ngx_brunzip_module/blob/master/ngx_http_brunzip_filter_module.c#L485</a><br>
<br>
Further, the test in question looks incorrect, as it doesn't <br>
take into account the edge case when amount of the output returned <br>
by BrotliDecoderDecompressStream() exactly matches the output <br>
buffer size, so ctx->available_out is 0, but rc is not <br>
BROTLI_DECODER_RESULT_NEEDS_MORE_OUTPUT.<br>
<br>
As for the documentation, it looks like you are looking for the <br>
documentation of the code in the module.  You may want to re-read <br>
it to understand (and fix the copyright as requested above to <br>
admit that you aren't the one who wrote most of the code).  Some <br>
basics about buffers and chains can be found here:<br>
<br>
<a href="http://nginx.org/en/docs/dev/development_guide.html#buffer" rel="noreferrer" target="_blank">http://nginx.org/en/docs/dev/development_guide.html#buffer</a><br>
<br>
Some simplified example of a code to work with buffers reuse as <br>
used by the module can be found here:<br>
<br>
<a href="http://nginx.org/en/docs/dev/development_guide.html#http_body_buffers_reuse" rel="noreferrer" target="_blank">http://nginx.org/en/docs/dev/development_guide.html#http_body_buffers_reuse</a><br>
<br>
Other chapters, such as "Code style", might be helpful as well.  <br>
Also, don't hesitate to look into the code of the functions you <br>
are using, it often helps.<br>
<br>
-- <br>
Maxim Dounin<br>
<a href="http://mdounin.ru/" rel="noreferrer" target="_blank">http://mdounin.ru/</a><br>
_______________________________________________<br>
nginx mailing list<br>
<a href="mailto:nginx@nginx.org" target="_blank">nginx@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx" rel="noreferrer" target="_blank">http://mailman.nginx.org/mailman/listinfo/nginx</a><br>
</blockquote></div>
</blockquote></div>