[users@httpd] Weak-etag not being handled correctly - bug found in source code

[Ben Cooper <Be...@detica.com>]

I've been investigating why weak eTags aren't being handled properly,
specifically why we always get a status 200 when we include a weak eTag,
and only get a status 304 when we delete the weak eTag and use the last
modified date instead, or make the eTag strong.  I've found the bug:
it's because the W tag is upper case, and Apache seems to assume all
headers are lower case.  However, I don't know where the best place is
to fix the bug, as discussed below.  

 

The bug is in the source code as follows:-

 

The response is evaluated in http_protocol in a function called
ap_meets_conditions(), as follows

http_protocol.c: v2.2.16, line 355

not_modified = ap_find_list_item(r->pool, if_nonematch, etag);

 

This line of code is executed for both strong and weak eTags for full
Get requests (ie not Range requests, which are caught by a previous
if-statement).  The call to ap_find_list_item is used to compare the
etag in the resource to the one in the request (held in a hash table
pointed to by the char *if_nonematch)

 

Ap_find_list_item exits in the util.c file.

util.c:  v2.2.16, line 1372

good = good && (*pos++ == apr_tolower(*ptr));

 

The for-loop surrounding this line of code walks through the characters
in the two strings, comparing them character-by-character.  It maintains
some state variables, such as in_qstr that tracks whether or not the
pointer is currently within a quote.  If it is within a quote, it
compares the two characters directly; if not within quotes, it executes
line 1372 that lower-cases *ptr value.  This assumes that the *pos value
is already lower case.  However, the weak eTag is defined by protocol as
beginning with the uppercase letter W, and it appears that uppercase W
is indeed what is stored in the hash table pointed to by if_nonematch
and pos.  

 

Testing this, my colleagues have created eTags with values such as 

"W1234-123123"

W/"1234-123123"

w/"1234-123123"

 

We have found that provided the eTag is fully quoted, or preceded by a
lowercase w, httpd works as we would expect and returns a 304 to a
conditional Get.  However, if the eTag is preceded by an uppercase W, as
per the protocol, then httpd always returns 200.  It appears as if the
cache is not working, but that is not the problem - it is the comparison
of the W that is failing and causing the behaviour.

 

A targeted fix is to lowercase if_nonematch just prior to calling
ap_find_list_item().  Because if_nonematch is a pointer, this may have
an undesirable side-effect elsewhere in the code, so a "safer" approach
is first to copy the string into a local variable, then lowercase the W,
and then call ap_find_list_item().  However, I wonder if a more natural
home for the fix would be to make the W lowercase when the hash table is
loaded?  

 

Could someone who knows the code much better than me take up this issue,
decide the best location for a fix, and implement it?  

Many thanks,

Ben Cooper


Please consider the environment before printing this email.

This message should be regarded as confidential. If you have received this email in error please notify the sender and destroy it immediately.
Statements of intent shall only become binding when confirmed in hard copy by an authorised signatory.  The contents of this email may relate to dealings with other companies within the Detica Limited group of companies.

Detica Limited is registered in England under No: 1337451.

Registered offices: Surrey Research Park, Guildford, Surrey, GU2 7YP, England.

Re: [users@httpd] Weak-etag not being handled correctly - bug found in source code

["William A. Rowe Jr." <wr...@rowe-clan.net>]

Such posts are really dev issues, not peer user support issues :)

Redirecting you to dev@httpd

On 10/8/2010 12:12 PM, Ben Cooper wrote:
> I’ve been investigating why weak eTags aren’t being handled properly, specifically why we
> always get a status 200 when we include a weak eTag, and only get a status 304 when we
> delete the weak eTag and use the last modified date instead, or make the eTag strong. 
> I’ve found the bug: it’s because the W tag is upper case, and Apache seems to assume all
> headers are lower case.  However, I don’t know where the best place is to fix the bug, as
> discussed below. 
> 
>  
> 
> The bug is in the source code as follows:-
> 
>  
> 
> The response is evaluated in http_protocol in a function called ap_meets_conditions(), as
> follows
> 
> http_protocol.c: v2.2.16, line 355
> 
> not_modified = ap_find_list_item(r->pool, if_nonematch, etag);
> 
>  
> 
> This line of code is executed for both strong and weak eTags for full Get requests (ie not
> Range requests, which are caught by a previous if-statement).  The call to
> ap_find_list_item is used to compare the etag in the resource to the one in the request
> (held in a hash table pointed to by the char *if_nonematch)
> 
>  
> 
> Ap_find_list_item exits in the util.c file.
> 
> util.c:  v2.2.16, line 1372
> 
> good = good && (*pos++ == apr_tolower(*ptr));
> 
>  
> 
> The for-loop surrounding this line of code walks through the characters in the two
> strings, comparing them character-by-character.  It maintains some state variables, such
> as in_qstr that tracks whether or not the pointer is currently within a quote.  If it is
> within a quote, it compares the two characters directly; if not within quotes, it executes
> line 1372 that lower-cases *ptr value.  This assumes that the *pos value is already lower
> case.  However, the weak eTag is defined by protocol as beginning with the uppercase
> letter W, and it appears that uppercase W is indeed what is stored in the hash table
> pointed to by if_nonematch and pos. 
> 
>  
> 
> Testing this, my colleagues have created eTags with values such as
> 
> “W1234-123123”
> 
> W/”1234-123123”
> 
> w/”1234-123123”
> 
>  
> 
> We have found that provided the eTag is fully quoted, or preceded by a lowercase w, httpd
> works as we would expect and returns a 304 to a conditional Get.  However, if the eTag is
> preceded by an uppercase W, as per the protocol, then httpd always returns 200.  It
> appears as if the cache is not working, but that is not the problem – it is the comparison
> of the W that is failing and causing the behaviour.
> 
>  
> 
> A targeted fix is to lowercase if_nonematch just prior to calling ap_find_list_item(). 
> Because if_nonematch is a pointer, this may have an undesirable side-effect elsewhere in
> the code, so a “safer” approach is first to copy the string into a local variable, then
> lowercase the W, and then call ap_find_list_item().  However, I wonder if a more natural
> home for the fix would be to make the W lowercase when the hash table is loaded? 
> 
>  
> 
> Could someone who knows the code much better than me take up this issue, decide the best
> location for a fix, and implement it? 
> 
> Many thanks,
> 
> Ben Cooper
> 
> Please consider the environment before printing this email.
> 
> This message should be regarded as confidential. If you have received this email in error please notify the sender and destroy it immediately.
> Statements of intent shall only become binding when confirmed in hard copy by an authorised signatory.  The contents of this email may relate to dealings with other companies within the Detica Limited group of companies.
> 
> Detica Limited is registered in England under No: 1337451.
> 
> Registered offices: Surrey Research Park, Guildford, Surrey, GU2 7YP, England.
> 
>