[Sugar-devel] [PATCH] webactivity: seed the XS cookie at startup

[Simon Schampijer]

Martin Langhoff wrote:
> On Sat, Feb 14, 2009 at 9:11 AM, Simon Schampijer <simon at schampijer.de> wrote:
>> Please find attached the patch against master.
> 
> Looks good to me (but I know nothing of what's changed in master...)
> 
>> - i use the backup_url to see if we are associated with a schoolserver
>> - why did you use the jabber server for this 'xs_fqdn = prof.jabber_server'?
> 
> Good question. Neither is the right one. In a XS driven net, both are
> equal. In a XS-on-the-internet situation, the "public XS" may decide
> to not offer backup service. Of the 3 (moodle/webapps, xmpp, backup),
> backup is the most burdensome on the server.
> 
> So I think there is a (very marginal) advantage to using the jabber
> server. But the most important hting is that 0.82.x and master use the
> same, so whatever you do, both should use the same...

Our registration URL is REGISTER_URL = 'http://schoolserver:8080/', 
wouldn't the right Domain than be 'schoolserver'? Since the cookie is 
about the registration with the schoolserver this makes most sense to me 
(the jabber server could be somewhere else).

> (The right fix is to have a 'schoolserver fqdn'  entry in the
> profile... but that's for the next Sugar dev cycle I guess...)
> 
>> - c.execute('''CREATE TABLE IF NOT EXISTS
>> +                         moz_cookies
>> +                         (id INTEGER PRIMARY KEY,
>> +                          name TEXT,
>> +                          value TEXT,
>> +                          host TEXT,
>> +                          path TEXT,
>> +                          expiry INTEGER,
>> +                          lastAccessed INTEGER,
>> +                          isSecure INTEGER,
>> +                          isHttpOnly INTEGER);''')
>>
>> - is the ';' correct here or a typo?
> 
> typo

ok;

>> - i only except for sqlite3.Error
> 
> Is that the only thing that could go wrong? My thinking has been: if
> we fail, let the startup succeed. This is a good feature, but not a
> showstopper.
> 
>> - what bothers me a bit is that you don't get an error when the database
>> does not exist - sqlite creates a new one actually - so we might return as
>> well on 'if not os.path.exists(os.path.join(_profile_path,
>> 'cookies.sqlite'))'

Well, all the calls in the try block are sqlite3 ones - if they fail - 
we catch it. If something else goes wrong - we want to fail and not hide ;p

> The DB does not exist on the first use of Browse. Actually, it does
> not get created until the first website sets the first cookie, AFAICS.
> 
> That means that on the first use of Browse the user goes to the XS and
> doesn't get autenticated. So if the DB doesn't exist, _we want to
> create it_. It's not a failure, it's success.

Ok, sounds good.

>> - the method could even be a function as it does not interact at all with
>> the class itself, not sure what is nicer
> 
> I'd prefer a function, but it's not my codebase, so follow the style... :-)

Done.

BTW: Is there a spec you used for the cookie format? I find field 
descriptions like expires - you name it expiry.

Thanks,
    Simon