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