Hey Shuxin, > I took some time to read your code, I believe I fully understand your > code at this moment. > My implementation is slightly differently from yours, and we have different > use-cases.
Thanks! > Here is my $0.02 value of comment to your code (cosmetic wise): > - the verb "support" in "trailer support" in too general. The > trailer-support in my mind include > a long list of features (see bellow) It's "trailer support" for the web serving part of NGINX, just not proxying. > - the "get" in "ngx_http_chunked_get_trailers" is sort of misleading. > The "get" here actually > means "generate". I thought you are looking for a existing buffer. That function name got changed a few times, since I cannot find one that I would be 100% happy with... Any suggestions? > Now go back to the long list of trailer support, I believe it needs to > cover at least: > 1) generate trailer headers and sent to downstream/visitor as you just > did > 2) reverse proxy propagate trailer headers from upstream, which needs > 2.0 parse the incoming trailer header, both considering the body is > buffered and not buffered, > 2.1. make the incoming trailer accessible via variable after content > phase, > 2.2. if response body is is not buffered, combine incoming trailer > header with the generated headers. > 2.3. convert incoming trailer to regular header if response body is > buffered > > 3|4|..|n) other features. > > I think 2.x is much harder than 1). I only implement 1) and 2.0) and > 2.1). While implement 2.1), I left some > room for 2.2 and 2.3 for the future. Now that you have plan for 2.x as your > email suggests, wouldn't it > be nice to submit these code first, and then go ahead with the code of 1). I > believe to support 2.2 or 2.3, > your existing code in chunk module needs lots of change. I don't see how anything you listed in 2.x would require modifications to the code I submitted... Actually, everything you listed should be handled in upstream and/or proxy modules, not chunking module, and it could easily use the API (r->headers_out.trailers) I want to add in this commit. Best regards, Piotr Sikora _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
