Re: [access-control] Implementation comments

Jonas Sicking wrote:
>>>> Looks great. The only thing I'd add is to be more explicit around the
>>>> initial description of the cache that each cache entry always has
>>>> exactly one of 'method' and 'header' empty and the other non-empty.
>>>> I.e. that either of them always exist, but never both.
>>>
>>> Ok, will fix that tomorrow. Got to go now.
>>
>> Added a note explaining the fields are mutually exclusive. Would it be 
>> useful to note that (origin, url, method, header) form the primary key 
>> of a cache entry? Since it's never needed by the specification in that 
>> way I didn't add it, but let me know.
> 
> Yes, I think it would be helpful to add that information. It wasn't 
> clear that the credentials flag wasn't part of the key until you put it 
> this way (though the spec already clearly says so, just easy to miss).

Actually, turns out this is not clear in the spec currently. In step 3 
of "To update cache entries means to follow this set of steps" it says:

For each method in methods (see preflight request) for which there is a 
/method cache match/ set the expirty time field value of the matching 
entry to expiry time.

The definition for /method cache match/ is:
when there is a cache entry for which there is a /cache match/ where the 
method field value is identical (case-sensitive) to the given method.

And the definition of /cache match/ requires that the credentials flag 
matches when true.

So in this case the credentials flag is actually part of the primary 
key. I.e. the spec says to not update an existing entry if a request is 
made with the credentials flag set to true, but the cache contains an 
entry with the credentials flag set to false. Instead a new entry should 
be created which will only differ in the value of the credentials flag 
(and possibly in the value of the expiry time).

I suspect the simplest solution is to actually make the credentials flag 
part of the primary key everywhere.

/ Jonas

Received on Monday, 29 September 2008 22:38:48 UTC