litmus locks:20 fix for support negate lock and invalid etag

[jeremydenoun]

When I try to check webdav compliance to litmus 0.13 (http://www.webdav.org/neon/litmus/)
with PDO mysql locks, the locks check 20 return :
20. fail_complex_cond_put. FAIL (PUT with complex bogus conditional should fail with 412: 204 No Content)
(and test content is updated)

After analysis I understand this is linked to If: header parsing especially with Not token...
My patch propose to add a condition for check this case and validate the test...

I try to find in git history how you can validate this litmus test but it's seems old and not update from long time ago

If you see a mistake on my side, please tell me

[evert]

Hi Jeremy,

I actually believe that this is a bug in the Litmus tool. I posted this message on their mailing list a while back:

http://lists.manyfish.co.uk/pipermail/litmus/2015-February/000350.html

Can you read that message and tell me if I misunderstood? I never got a response.

[jeremydenoun]

I read your message and I agree except I think in this case we have :
If: ( valid-locktoken AND invalid-etag) OR (NOT (empty-locktoken=>valid AND
invalid-etag))

but this don't change your analysis
If: (true AND false) OR (NOT (true AND false)) => true

except if we consider unlogical parenthesis
If: ( valid-locktoken AND invalid-etag) OR ((NOT empty-locktoken/valid) AND
invalid-etag)
evaluate :
If: (true AND false) OR ((NOT true) AND false))
=> false

it's not an usual case and it's seems code/logic (in sabre and litmus) don't change since 2 years...
it's a little weird

[evert]

Hmm... you are saying that this:

(Not <DAV:no-lock> ["c3d40b11121145ec31f5c1acc078b658"])

Means:

(Not <DAV:no-lock>) AND (["c3d40b11121145ec31f5c1acc078b658"])

Instead of:

Not (<DAV:no-lock> AND ["c3d40b11121145ec31f5c1acc078b658"])

I need to re-read the specification to see if you're right or not =)

[jeremydenoun]

If this can help you http://www.webdav.org/specs/rfc4918.html#if.header.evaluation.example.no-tag
10.4.7 / 10.4.7 but with etag in second part

This is little unclear and keep reasonable doubt

[evert]

The spec does indeed say:

A Condition that consists of a single entity-tag or state-token evaluates to true if the resource matches the described state (where the individual matching functions are defined below in Section 10.4.4). Prefixing it with "Not" reverses the result of the evaluation (thus, the "Not" applies only to the subsequent entity-tag or state-token).

[evert]

The problem I see with the proposed fix is that I don't think it correctly fixes the issue.

We basically now need to accommodate this as well:

(Not <DAV:no-lock> NOT ["c3d40b11121145ec31f5c1acc078b658"])

Two NOT, and our data structure does not yet allow this. This will be a breaking change, so I'm tying it to the future 3.1 release and opening a new issue for this.