After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 791365 - Ephy fails to get storage credentials for sync
Ephy fails to get storage credentials for sync
Status: RESOLVED OBSOLETE
Product: epiphany
Classification: Core
Component: Sync
3.26.x
Other Linux
: Normal critical
: ---
Assigned To: Gabriel Ivașcu
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-12-08 00:03 UTC by Sergio Villar
Modified: 2018-08-03 21:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use g_ascii_dtostr() instead of g_strdup_printf() for 64-bit integers (7.10 KB, patch)
2018-03-02 11:22 UTC, Gabriel Ivașcu
rejected Details | Review

Description Sergio Villar 2017-12-08 00:03:49 UTC
I'm running Debian's 3.26.2 (WebKitGtk 2.18.3)

Ephy continuously complains about an error getting the storage credentials for sync. I got literally dozens of error banners about that, it makes ephy totally unusable as all those errors completely fill up the screen.

BTW this has nothing to do with a previously reported bug about sync not connection aware as this happens with pretty stable wired connections.
Comment 1 Gabriel Ivașcu 2017-12-08 06:48:01 UTC
Thanks for reporting. I'll investigate this.

Do you get dozens or error notifications at once, or they accumulate over time, like bug 790094 says?

Can you please run Epiphany from terminal and show me a paste with all the logs and warning messages?
Comment 2 Michael Catanzaro 2017-12-11 00:34:42 UTC
(In reply to Gabriel Ivașcu from comment #1)
> Do you get dozens or error notifications at once, or they accumulate over
> time, like bug 790094 says?

I'm planning to release 3.26.4 tomorrow, so let me know if you want me to delay for this, but I guess it's probably already fixed by bug #790286 (which Sergio is missing because Debian never updated to 3.26.3) and bug #790094.
Comment 3 Gabriel Ivașcu 2017-12-11 11:20:32 UTC
(In reply to Michael Catanzaro from comment #2)
> I'm planning to release 3.26.4 tomorrow, so let me know if you want me to
> delay for this, but I guess it's probably already fixed by bug #790286
> (which Sergio is missing because Debian never updated to 3.26.3) and bug
> #790094.

It's hard to say, because I cannot reproduce this. But no internet connectivity would definitely result in a behavior like this, since the communication with the server wouldn't work - and this would happen again and again, at every sync.

However, I'd like to have the patch in bug 790819 present in the next release. I'll merge it soon.
Comment 4 Sergio Villar 2017-12-28 20:45:11 UTC
(In reply to Gabriel Ivașcu from comment #1)
> Thanks for reporting. I'll investigate this.
> 
> Do you get dozens or error notifications at once, or they accumulate over
> time, like bug 790094 says?

No, I got dozens at once.

> Can you please run Epiphany from terminal and show me a paste with all the
> logs and warning messages?

I'll try, don't have much time ATM so let's see
Comment 5 Link Dupont 2018-02-05 16:02:40 UTC
Running Epiphany 3.26.5.1, I'm getting this error ("Failed to obtain storage credentials") once every time I start up Epiphany. When run from Terminal, I get a slightly more verbose error:

** (epiphany:29253): WARNING **: Failed to obtain storage credentials. Status code: 401, response: {"status": "invalid-timestamp", "errors": [{"location": "body", "name": "", "description": "Unauthorized"}]}

I can re-enter my password in the Accounts form, it then immediately "succeeds" (I get the notification about "now syncing") and then it drops back to the login form and presents this error message. I can do this repeatedly with the same behavior.
Comment 6 Michael Catanzaro 2018-02-05 16:51:15 UTC
(In reply to Link Dupont from comment #5)
> Running Epiphany 3.26.5.1, I'm getting this error ("Failed to obtain storage
> credentials") once every time I start up Epiphany. When run from Terminal, I
> get a slightly more verbose error:
> 
> ** (epiphany:29253): WARNING **: Failed to obtain storage credentials.
> Status code: 401, response: {"status": "invalid-timestamp", "errors":
> [{"location": "body", "name": "", "description": "Unauthorized"}]}

Aha, good find!
Comment 7 Sudarshan Kakoty 2018-02-28 16:15:56 UTC
(In reply to Link Dupont from comment #5)
> Running Epiphany 3.26.5.1, I'm getting this error ("Failed to obtain storage
> credentials") once every time I start up Epiphany. When run from Terminal, I
> get a slightly more verbose error:
> 
> ** (epiphany:29253): WARNING **: Failed to obtain storage credentials.
> Status code: 401, response: {"status": "invalid-timestamp", "errors":
> [{"location": "body", "name": "", "description": "Unauthorized"}]}
> 
> I can re-enter my password in the Accounts form, it then immediately
> "succeeds" (I get the notification about "now syncing") and then it drops
> back to the login form and presents this error message. I can do this
> repeatedly with the same behavior.

I am running Debian Unstable, and I think this problem is resolved in Epiphany 3.26.6, WebKitGTK+ 2.18.6.

Are you still getting this issue?
Comment 8 Michael Catanzaro 2018-02-28 19:13:53 UTC
There are no patches in 3.26.6 that might have fixed this, so let's keep this open.

I wonder why the timestamp is invalid. Maybe we need to add more debugging for that.
Comment 9 Link Dupont 2018-02-28 19:18:07 UTC
I only see this bug on my workstation. My laptop (also running the same Fedora packages) does not exhibit this issue. That leads me to believe it's environmental. Some sort of connectivity check maybe?
Comment 10 Gabriel Ivașcu 2018-03-01 09:32:54 UTC
(In reply to Link Dupont from comment #5)

> ** (epiphany:29253): WARNING **: Failed to obtain storage credentials.
> Status code: 401, response: {"status": "invalid-timestamp", "errors":
> [{"location": "body", "name": "", "description": "Unauthorized"}]}

From this error message seems that there is a problem with the timestamp inside the request with the BrowserID assertion sent to the Token Server. I'm fairly positive that the place to look is ephy_sync_crypto_create_assertion() in ephy-sync-crypto.c (https://git.gnome.org/browse/epiphany/tree/lib/sync/ephy-sync-crypto.c#n941). But I'm not sure what happens with the timestamp such that is considered invalid, and I think we can't find out unless we print out the actual request sent to the server. Would you be willing to test a patch with some extra debug messages, since this reproduces every time on your machine?
Comment 11 Sudarshan Kakoty 2018-03-01 09:40:18 UTC
(In reply to Gabriel Ivașcu from comment #10)
> (In reply to Link Dupont from comment #5)
> 
> > ** (epiphany:29253): WARNING **: Failed to obtain storage credentials.
> > Status code: 401, response: {"status": "invalid-timestamp", "errors":
> > [{"location": "body", "name": "", "description": "Unauthorized"}]}
> 
> From this error message seems that there is a problem with the timestamp
> inside the request with the BrowserID assertion sent to the Token Server.
> I'm fairly positive that the place to look is
> ephy_sync_crypto_create_assertion() in ephy-sync-crypto.c
> (https://git.gnome.org/browse/epiphany/tree/lib/sync/ephy-sync-crypto.
> c#n941). But I'm not sure what happens with the timestamp such that is
> considered invalid, and I think we can't find out unless we print out the
> actual request sent to the server. Would you be willing to test a patch with
> some extra debug messages, since this reproduces every time on your machine?

As far as my knowledge, this types of issues usually caused by huge difference of the client/server clocks, than allowed value. Let me see whats going on there... :)
Comment 12 Gabriel Ivașcu 2018-03-01 09:42:12 UTC
(In reply to Gabriel Ivașcu from comment #10)
> I'm fairly positive that the place to look is
> ephy_sync_crypto_create_assertion() in ephy-sync-crypto.c
> (https://git.gnome.org/browse/epiphany/tree/lib/sync/ephy-sync-crypto.
> c#n941).

Should have probably linked to the code in gnome-3-26 branch (https://git.gnome.org/browse/epiphany/tree/lib/sync/ephy-sync-crypto.c?h=gnome-3-26#n921) since master is more likely to change in the future.
Comment 13 Link Dupont 2018-03-02 03:42:57 UTC
(In reply to Gabriel Ivașcu from comment #10)
> Would you be willing to test a patch with
> some extra debug messages, since this reproduces every time on your machine?

I'd be happy to. It would be easier if you could patch against the 3.26 branch, since that's the stack I have installed on that machine.
Comment 14 Gabriel Ivașcu 2018-03-02 10:00:00 UTC
Hmmm I just read this from GLib's docs (https://gitlab.gnome.org/GNOME/glib/blob/master/glib/docs.c#L452-464):

"""Some platforms do not support scanning and printing 64-bit integers, even though the types are supported. On such platforms %G_GUINT64_FORMAT is not defined.  Note that scanf() may not support 64-bit integers, even if %G_GINT64_FORMAT is defined. Due to its weak error handling, scanf() is not recommended for parsing anyway; consider using g_ascii_strtoull() instead."""

This seems like a very possible cause for our error. (Are you by any chance running a 32-bit system on your workstation?) I think we should replace all g_strdup_printf() occurrences that take (u)int64s with g_ascii_strtoull().
Comment 15 Gabriel Ivașcu 2018-03-02 10:13:27 UTC
(In reply to Gabriel Ivașcu from comment #14)
> This seems like a very possible cause for our error. (Are you by any chance
> running a 32-bit system on your workstation?) I think we should replace all
> g_strdup_printf() occurrences that take (u)int64s with g_ascii_strtoull().

Actually, it should be g_ascii_dtostr() to convert from number to string.
Comment 16 Gabriel Ivașcu 2018-03-02 11:22:31 UTC
Created attachment 369180 [details] [review]
Use g_ascii_dtostr() instead of g_strdup_printf() for 64-bit integers

From GLib's docs: "Some platforms do not support scanning and printing
64-bit integers, even though the types are supported. On such plaftorms,
G_GUINT64_FORMAT is not defined. Note that scanf() may not support 64-bit
integers, even if G_GINT64_FORMAT is defined. Due to its weak error
handling, scanf() is not recommended for parsing anyway; consider using
g_ascii_strtoull() instead."
Comment 17 Gabriel Ivașcu 2018-03-02 11:25:48 UTC
Let's see how this patch turns out, maybe it fixes the problem. I made it for the gnome-3-26 branch, but it applies just fine to master too.
Comment 18 Michael Catanzaro 2018-03-02 15:05:30 UTC
(In reply to Gabriel Ivașcu from comment #16)
> Created attachment 369180 [details] [review] [review]
> Use g_ascii_dtostr() instead of g_strdup_printf() for 64-bit integers
> 
> From GLib's docs: "Some platforms do not support scanning and printing
> 64-bit integers, even though the types are supported. On such plaftorms,
> G_GUINT64_FORMAT is not defined."

That's nuts. :P I really doubt this is the problem.

And I assure you that WebKit (and probably a bunch of other software we depend on) assumes that printf can handle 64-bit integers; there's no need to alter epiphany to avoid that. ephy_uint64_to_string is a lot harder than writing "%"PRIu64. Most developers would be lazy and write %lu, which really *can* cause problems on some platforms, e.g. on Mac where %llu is needed.

"%"PRIu64 is guaranteed to work; the build would fail if that were not available.

Note: I see you were using "%"PRId64 rather than "%"PRIu64 to print unsigned ints. That's the first thing I would change. That's surely not your problem either, but can't hurt to fix it.
Comment 19 Michael Catanzaro 2018-03-02 15:06:50 UTC
Link, even though I've marked this patch as rejected, it's still worth trying out, just in case it actually does fix the problem (which would surprise me, but if so, that indicates PRId64 may be to blame).
Comment 20 Michael Catanzaro 2018-03-02 15:10:34 UTC
(In reply to Gabriel Ivașcu from comment #14)
> (Are you by any chance running a 32-bit system on your workstation?)

This was a good question, though. I've seen lots of bugs recently that only happen on 32-bit systems, because the wrong int type is used somewhere.
Comment 21 Gabriel Ivașcu 2018-03-02 15:22:30 UTC
(In reply to Michael Catanzaro from comment #18)
> Note: I see you were using "%"PRId64 rather than "%"PRIu64 to print unsigned
> ints. That's the first thing I would change. That's surely not your problem
> either, but can't hurt to fix it.

The "%"PRId64s are legit in all those places. They are used for gint64 variables only. There is a single place where guint64 is used instead of gint64 as a type for a variable that holds a timestamp, and that is ephy_sync_crypto_create_assertion() in ephy-sync-crypto.c. Interestingly enough, that's the only place where a timestamp in milliseconds is required, as the rest of the server requests work with timestamps in seconds.
Comment 22 Link Dupont 2018-03-03 04:57:01 UTC
(In reply to Gabriel Ivașcu from comment #14)
> (Are you by any chance running a 32-bit system on your workstation?)

Nope, 64-bit. I'll give the patch a test and see what I find out.
Comment 23 Michael Catanzaro 2018-03-27 15:25:27 UTC
(In reply to Link Dupont from comment #22)
> (In reply to Gabriel Ivașcu from comment #14)
> > (Are you by any chance running a 32-bit system on your workstation?)
> 
> Nope, 64-bit. I'll give the patch a test and see what I find out.

Did you get a chance to try it?
Comment 24 Link Dupont 2018-03-28 03:30:22 UTC
(In reply to Michael Catanzaro from comment #23)
> Did you get a chance to try it?

I just logged into Sync again on that system, and now the error message doesn't appear. ಠ_ಠ
Comment 25 Sergio Villar 2018-03-29 08:23:56 UTC
Same here. I've recently updated to:

3.28.0.1
2.20
Comment 26 Link Dupont 2018-03-29 14:05:02 UTC
Ok, it started happening again. I have patched and built epiphany-3.26.6. I'll update when/if I get some logs.
Comment 27 GNOME Infrastructure Team 2018-08-03 21:23:43 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/epiphany/issues/446.