Set built in etag when using ETag as well #213

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
2 participants
Member

bakins commented Mar 8, 2013

AFAIK, the correct header we should be using is ETag. This sets the built in when that is used and leaves "E-Tag" as is.

Owner

agentzh commented Mar 8, 2013

Thank you for the patch? Could you please add some test cases for this?

Member

bakins commented Mar 8, 2013

Sure. I'll do one that's expected to pass and one thats expected to "fail"

Owner

agentzh commented Mar 8, 2013

Perfect! Thanks!

Member

bakins commented Mar 9, 2013

I forgot that etags are only supported, without patches, in 1.3.x, so I can't really write a very good test for 1.2. This may can be set aside until 1.3 is stable.

Owner

agentzh commented Mar 9, 2013

Hello!

On Fri, Mar 8, 2013 at 8:17 PM, Brian Akins notifications@github.com wrote:

I forgot that etags are only supported, without patches, in 1.3.x, so I can't really write a very good test for 1.2. This may can be set aside until 1.3 is stable.

Actually you can use the "--- skip_nginx" test directive here to skip
such test cases for nginx versions older than a particular 1.3.x
version:

http://search.cpan.org/perldoc?Test::Nginx::Socket#skip_nginx

Best regards,
-agentzh

Member

bakins commented Mar 9, 2013

Didn't know that. I'll give that a try. Thanks!

agentzh added a commit that referenced this pull request Apr 21, 2013

bugfix: setting ngx.header.etag could not affect other things reading…
… the ETag response header (like the "etag" directive introduced in nginx 1.3.3+). thanks Brian Akins for the patch in #213.
Owner

agentzh commented Apr 21, 2013

After fixing various minor issues in your tests, I've merged your patch into master :) Thanks for your contribution!

@agentzh agentzh closed this Apr 21, 2013

@bakins bakins deleted the bakins:etag branch Apr 21, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment