[Bug 33404] Move caps hash stuff to Wocky

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=33404

Jonny Lamb <jonny.lamb at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|r-                          |

--- Comment #11 from Jonny Lamb <jonny.lamb at collabora.co.uk> 2011-03-11 07:27:12 PST ---
(In reply to comment #10)
> I don't think this is right, I'm afraid. The XEP says:
> 
>   • For each field other than FORM_TYPE:
>       ‣ Append the value of the "var" attribute, followed by the '<'
>         character.
>       ‣ Sort values by the XML character data of the <value/> element.
>       ‣ For each <value/> element, append the XML character data,
>         followed by the '<' character.
> 
> If WockyDataForm's API doesn't make this possible without contortions, we
> should make it possible, rather than risking incorrectly calculating the hash
> (despite having had all the necessary information at some point).

Okay, so I thought it would be difficult to start supporting all the other
field types, but it turned out to be fine. Fixed and added a test for booleans.

One problem I see might be an issue here is that the data form parser treats
both "true" and "1" as value booleans for TRUE (and the opposite for FALSE),
but the caps hashing code only gets a GValue back and uses 1 and 0 for TRUE and
FALSE respectively. We don't get the actual stanza from the data form field so
can't check the actual value.

Also, it switches on the WockyDataFormFieldType value. I'm wondering if a good
default value is to act as if it's text-single? What do you think?

> I think this is wrong, too. We should fail to calculate the hash in this
> case. Arguably parsing the data form should fail if there's no FORM_TYPE
> field, but XEP-0004 doesn't actually mandate that that field exist,
> afaict.
> 
> I think there should be a test for the cases where a form has no
> FORM_TYPE,

Done.

> ...and where the disco reply contains two forms with the same
> FORM_TYPE; in both cases, calculating the hash should fail.

Done.

> We should not cache the supposed hash negatively, though: otherwise,
> malicious contacts could trick us into negatively caching a hash that
> some other client is using correctly and legitimately. I think it's
> reasonable for Gabble to still use the capabilities, though.

When gabble gets a reply from a disco request that doesn't match the hash we
originally received (including us not being able to hash it ourselves) it sets
the trust of the hash back to zero but doesn't appear to do much more than
that?

> From jdev@:
> > Zash: variations of empty fields isn't specified well iirc
> > Zash: eg <field><value/></field> vs <field/> vs no field at all
> 
> Might be worth testing the first two variants do something sensible.

Okay I added tests for <field var='lol'><value/></field>, <field var='lol'/>
and <field/>. The first two fail because of the presence of the var attribute,
but the last one fails because the data form parser just ignores fields with no
var attribute.

> +  WockyStanza *  stanza = wocky_stanza_build (WOCKY_STANZA_TYPE_IQ,
> 
> What's going on ^^ here?

Fixed.

Check out my updated branch.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.