Re: [access-control] update from the editor

On Thu, 10 May 2007 22:10:23 +0200, Ian Hickson <ian@hixie.ch> wrote:
> On Thu, 10 May 2007, Anne van Kesteren wrote:
>> On Wed, 09 May 2007 21:28:12 +0200, Ian Hickson <ian@hixie.ch> wrote:
>>>
>>> In 2.1, ""deny" rules can be used by authors to deny read access from
>>> external resources to the entire server a simple way without having to
>>> check each individual XML resource that may have <?access-control?>
>>> processing instructions specified." is somewhat confusing to a first
>>> time reader because the PI hasn't yet been met.
>>>
>>> In fact it's still confusing to me now. I think your prepositions are
>>> all wrong. I'm not really sure what you're trying to say.
>>
>> I tried to clarify it.
>
> Yeah, that's better. It might help readers understand the sentence more  
> if you explicitly point out the assumption that headers can be set  
> across the entire server even if files can't be modified across the  
> entire server.

Ok, done.


>>> In 3: "The match list and exclude list are both unordered lists of
>>> access items." -- "the" match list? "the" exclude list? There are 3 of
>>> each! This should probably be in the plural or something.
>>
>> Made the definitions plural.
>
> That broke the cross-reference to "exclude list". :-(

Indeed, fixed!


> One thing I didn't notice before -- you have "for each
> Content-Access-Control header and rule within" but shouldn't that just be
> for each rule, not for each header and rule? Surely you got rules when  
> you parsed the headers, so you don't need to do anything for each header
> anymore at this point.

Makes sense, changed.


>>> "user agents must grant access to the resource" can we make that a
>>> SHOULD instead of a MUST?
>>
>> Makes sense, addressed.
>
> Looks good. It's sort of weird now because the MUST for the deny is at  
> the top of section 3, but the SHOULDs override it in the algorithm, but I
> guess that's ok.

Yeah, this is why the algorithm mentions "(unless stated otherwise)" at  
the top.


> The note "This means that the attribute value can not be the empty
> string." seems to be wrong -- why does it mean that?

I removed the note, but clarified the requirements to say that if no value  
was obtained it would return in a failure as well (this matches the  
requirements on authors).


>>> I can't really comment on the "match" algorithm because I don't know
>>> what Request URL is supposed to be. For example, is it expected to be
>>> an absolute URL always, or can it be relative? What does it mean for
>>> the origin not to have a scheme? Why would you ignore the scheme if
>>> it's not followed by "://" ? How can it not have a port? Are
>>> non-host-based- authority schemes allowed?
>>>
>>> Step 9 doesn't specify the order.
>>
>> I tried to fix these as well.
>
> Step 1 is backwards (are you setting request URL or origin?).

Which step 1, exactly? I'm not sure I see the problem.


> The definition of requestURL doesn't handle request URLs that are not
> host-based authorities (e.g. data: URLs). I assume you just auto-deny for
> those? Or, as the note says, maybe it should be based on origin? Hm.

It would be nice to let those URIs still be able to access if the  
requested resource has:

   Content-Access-Control: allow <*>

I modified the definition accordingly.


> How can item be "*"? Doesn't that not match the syntax?

It does match the syntax.


> How about if requestURL is http://example.com:81 and item is
> http://example.com -- should it be allowed? Even though the implicit port
> is 80, not 81?

I suppose we could default to 80 if the scheme is specified. Note that the  
scheme can be omitted as well in which case this would not be possible.  
What is desirable? This would probably also require a change to the syntax  
to allow * for port if we really want this.


> Step 5 would, given those examples, result in
>
>    "http://example", "com:81"
>    "http://example", "com"
>
> ...respectively, which I'm assuming isn't what you want (shouldn't they  
> be dropping the scheme:// and :port parts, if present?).

This should no longer happen.


> What does "equal" mean in step 7.4? Case-insensitive?

Changed to case-insensitive. Also defined that. Probably need to do some  
further checking on this.


> In step 7.4: "apply to these set of steps to the next list item" looks
> like it has an extra "to" or something.

Indeed, fixed.


-- 
Anne van Kesteren
<http://annevankesteren.nl/>
<http://www.opera.com/>

Received on Friday, 11 May 2007 12:28:30 UTC