WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
211 stars 135 forks source link

[wpe-2.38][soup3] cookie db schema change #1226

Closed emutavchi closed 5 months ago

emutavchi commented 9 months ago

wpe-2.38 comes with libsoup3 enabled by default. this brings http2 support and other improvements. but it also changes cookie database schema, adding new field sameSite:

soup2:

id INTEGER PRIMARY KEY, name TEXT, value TEXT, host TEXT, path TEXT,expiry INTEGER, lastAccessed INTEGER, isSecure INTEGER, isHttpOnly INTEGER

soup3:

id INTEGER PRIMARY KEY, name TEXT, value TEXT, host TEXT, path TEXT, expiry INTEGER, lastAccessed INTEGER, isSecure INTEGER, isHttpOnly INTEGER, sameSite INTEGER

this makes problematic to do a downgrade to previous version if device fw needs to be rolled back for any reason.

specifically cookie jar updates are failing with:

(process:27): libsoup-WARNING **: 15:58:15.693: Failed to execute query: table moz_cookies already exists                                                                                                                                                                                                                                                                                                                                                                                 
(process:27): libsoup-WARNING **: 15:58:15.693: Failed to execute query: table moz_cookies has 10 columns but 9 values were supplied 

I'm creating this issue to discuss possible fix/mitigation that can be done on wpewebkit level.

pgorszkowski-igalia commented 8 months ago

@emutavchi : the idea is to check existence and remove 'sameSite' column from the moz_cookies table when wpewebkit starts in case of libsoup2. libsoup2 is in 2.22 and optionally in 2.28, so implementation should be done in these two branches.

pgorszkowski-igalia commented 8 months ago

@emutavchi : what version of libsoup2 do you use? 'sameSite' column is since 2.74.0: https://gitlab.gnome.org/GNOME/libsoup/-/blob/2.74.0/libsoup/soup-cookie-jar-db.c#L131 So it should be available in version WPE 2.22

modeveci commented 8 months ago

@pgorszkowski-igalia As it is being experienced, the libsoup2 in use does not support "sameSite".

_" the idea is to check existence and remove 'sameSite' column from the mozcookies table when wpewebkit starts in case of libsoup2. libsoup2 is in 2.22 and optionally in 2.28, so implementation should be done in these two branches."

So you are saying, there should be some impl. in 2.22 and 2.28 to remove/or ignore sameSite column if DB has sameSite but libsoup2 in use, Is it something we can do in those branches?

pgorszkowski-igalia commented 8 months ago

@modeveci : WPE 2.22 uses libsoup in version 2.74.0 (and 2.28 if uses libsoup2 it also uses version 2.74) which contains sameSite column in moz_cookies table. So it means nothing has to be done at least for WPE 2.22 and 2.28. My question is for which version of WPE and libsoup2 the problem was observed?

modeveci commented 8 months ago

I see, @emutavchi for mitigation to be able to downgrade from 2.38 to one of 2.28 or 2.22, is it an option to use libsoup2 2.74 version, then there would be no change needed in DB files? What do you think?

emutavchi commented 8 months ago

@pgorszkowski-igalia we use libsoup that comes with Dunfell (https://git.openembedded.org/openembedded-core/tree/meta/recipes-support/libsoup/libsoup-2.4_2.68.4.bb?h=dunfell).

Upgrade to Kirkstone is in progress, which also updates libsoup to 2.74.2 (https://git.openembedded.org/openembedded-core/tree/meta/recipes-support/libsoup/libsoup-2.4_2.74.2.bb?h=kirkstone). So, I suppose we'll have this problem with wpe-2.28 now as well?

pgorszkowski-igalia commented 8 months ago

@pgorszkowski-igalia we use libsoup that comes with Dunfell (https://git.openembedded.org/openembedded-core/tree/meta/recipes-support/libsoup/libsoup-2.4_2.68.4.bb?h=dunfell).

libsoup in version 2.68.4 does not contain 'sameSite' column in table 'moz_cookies' so here the problem exists. https://gitlab.gnome.org/GNOME/libsoup/-/blob/2.68.4/libsoup/soup-cookie-jar-db.c#L131

Upgrade to Kirkstone is in progress, which also updates libsoup to 2.74.2 (https://git.openembedded.org/openembedded-core/tree/meta/recipes-support/libsoup/libsoup-2.4_2.74.2.bb?h=kirkstone). So, I suppose we'll have this problem with wpe-2.28 now as well?

If you use version version 2.74.2 from the Kirkstone then the problem will not be visible, because libsoup in version 2.74.2 contains 'sameSite' column in table 'moz_cookies'. https://gitlab.gnome.org/GNOME/libsoup/-/blob/2.74.2/libsoup/soup-cookie-jar-db.c?ref_type=tags#L131

emutavchi commented 8 months ago

@pgorszkowski-igalia I meant downgrade scenario, from Kirkstone back to Dunfell. Moving to '2.74.2' does "break" Cookie jar DB.

pgorszkowski-igalia commented 8 months ago

@emutavchi: yes, if you downgrade from Kirkstone to Dunfell then the problem will be observed. Do you assume such situation? Should I prepare fix for that case? For which version of WPE it should be implemented? 2.28? 2.38?

emutavchi commented 8 months ago

@pgorszkowski-igalia , I anticipate it will become a problem in near future. I suppose the fix should be libsoup side. if you plan to fix it in wpe-webkit, then we'll need a fix for wpe-2.28 and later

pgorszkowski-igalia commented 7 months ago

@emutavchi : I have checked and patch from https://gitlab.gnome.org/GNOME/libsoup/-/commit/d4f8cff260326a4688cf078d0e63fae30633f02c (the diff: https://gitlab.gnome.org/GNOME/libsoup/-/commit/d4f8cff260326a4688cf078d0e63fae30633f02c.diff) can be easily applied/cherry-picked on libsoup 2.68.4 (Dunfell). With this patch on libsoup 2.68.4, the downgrading from Kirkstone to Dunfell will not need to any changes. Would that work for you?

emutavchi commented 6 months ago

Hi @pgorszkowski-igalia we'd like to avoid unnecessary change of cookie jar db schema with Dunfell image. We're trying different solution, and will update here once we got results.

simi-mathew commented 5 months ago

We were able to resolve the issue with the cookie jar db schema change during downgrade from Kirkstone to Dunfell with the following patch

--- libsoup-2.68.4.orig/libsoup/soup-cookie-jar-db.c
+++ libsoup-2.68.4/libsoup/soup-cookie-jar-db.c
@@ -132,7 +132,7 @@ soup_cookie_jar_db_new (const char *file

 #define QUERY_ALL "SELECT id, name, value, host, path, expiry, lastAccessed, isSecure, isHttpOnly FROM moz_cookies;"
 #define CREATE_TABLE "CREATE TABLE moz_cookies (id INTEGER PRIMARY KEY, name TEXT, value TEXT, host TEXT, path TEXT,expiry INTEGER, lastAccessed INTEGER, isSecure INTEGER, isHttpOnly INTEGER)"
-#define QUERY_INSERT "INSERT INTO moz_cookies VALUES(NULL, %Q, %Q, %Q, %Q, %d, NULL, %d, %d);"
+#define QUERY_INSERT "INSERT INTO moz_cookies (id, name, value, host, path,expiry, lastAccessed, isSecure, isHttpOnly) VALUES(NULL, %Q, %Q, %Q, %Q, %d, NULL, %d, %d);"
 #define QUERY_DELETE "DELETE FROM moz_cookies WHERE name=%Q AND host=%Q;"

 enum {
modeveci commented 5 months ago

Thank you for update @simi-mathew. As it is being resolved, I am closing this ticket.