Bug 24818 - [GTK] http auth dialog pops up twice after a cancelled atempt
Summary: [GTK] http auth dialog pops up twice after a cancelled atempt
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk, Soup
Depends on:
Blocks:
 
Reported: 2009-03-25 15:44 PDT by Gustavo Noronha (kov)
Modified: 2009-07-21 03:52 PDT (History)
3 users (show)

See Also:


Attachments
log of soup messages up to when I click 'OK' on the first dialog (4.00 KB, text/plain)
2009-05-16 05:26 PDT, Gustavo Noronha (kov)
no flags Details
cancelauth.patch (2.42 KB, patch)
2009-05-21 23:53 PDT, Xan Lopez
mjs: review+
Details | Formatted Diff | Diff
drop-workaround.patch (2.08 KB, patch)
2009-07-20 15:49 PDT, Priit Laes (IRC: plaes)
zecke: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gustavo Noronha (kov) 2009-03-25 15:44:10 PDT
How to reproduce: I go to a site where http basic auth is required, and click cancel when the dialog appears. Then I click the location bar entry and press enter. Now two authentication dialogs appear, and I get this in the terminal:

(lt-GtkLauncher:2577): libsoup-CRITICAL **: soup_message_io_pause: assertion `io != NULL' failed

If I cancel both dialogs, I get this:

(lt-GtkLauncher:3399): libsoup-CRITICAL **: soup_message_io_unpause: assertion `io != NULL' failed

If I press OK on both dialogs, the site loads correctly.
Comment 1 Xan Lopez 2009-03-28 14:01:12 PDT
I guess we should we doing something in Cancel, but not sure what. CCing Dan.
Comment 2 Dan Winship 2009-03-28 15:27:44 PDT
webkitsoupauthdialog.c looks like it's doing the right thing; if the user cancels, you should just unpause the message without calling soup_auth_authenticate() on it. So this might be a libsoup bug.
Comment 3 Dan Winship 2009-03-28 15:39:32 PDT
or maybe not; i went to try to add a test for this case to libsoup/tests/auth-test.c, but there's already one there. so this should work.

things to figure out: is something else requeueing the first message? when the two auth dialogs pop up, are they for the same SoupMessage or for different ones? If the former: what is the state of the message queue after the first message fails? If the latter: is the feature getting attached to the session twice maybe?
Comment 4 Gustavo Noronha (kov) 2009-05-16 05:26:02 PDT
Created attachment 30412 [details]
log of soup messages up to when I click 'OK' on the first dialog

It's interesting to note that sometimes two dialogs pop, sometimes three. I'll investigate your other queries, Dan.
Comment 5 Gustavo Noronha (kov) 2009-05-16 05:39:13 PDT
(In reply to comment #4)
> Created an attachment (id=30412) [review]
> log of soup messages up to when I click 'OK' on the first dialog

Just to make it clear. This log contains: what happens 'till I click 'cancel', then I try loading the same uri again, and click 'OK' in one of the 2/3 dialogs.
Comment 6 Dan Winship 2009-05-17 07:37:56 PDT
Did you edit that at all besides adding "first try", "second try"? The Soup-Debug lines don't seem to match up right. Eg, the response for SoupMessage 0x83eede0 shows up in the log before the request. (This *might* be a SoupLogger bug?)

Another oddity is that there are two SoupSessions in the log (0x95ec970 for the first few requests, then 0x8108970 for the last one). Last I knew WebKit didn't (intentionally) create more than one SoupSession.

Are multiple threads being used here? SoupSessionAsync doesn't go to any effort to be threadsafe, so it's possible that that would mess things up. (In particular, if you called certain methods from a thread other than the thread whose GMainContext the session uses, then they might do bad things because they might queue an asynchronous operation, which could end up completing in the other thread even before the calling function returns.)
Comment 7 Gustavo Noronha (kov) 2009-05-21 08:37:59 PDT
(In reply to comment #6)
> Did you edit that at all besides adding "first try", "second try"? The
> Soup-Debug lines don't seem to match up right. Eg, the response for SoupMessage
> 0x83eede0 shows up in the log before the request. (This *might* be a SoupLogger
> bug?)
> 
> Another oddity is that there are two SoupSessions in the log (0x95ec970 for the
> first few requests, then 0x8108970 for the last one). Last I knew WebKit didn't
> (intentionally) create more than one SoupSession.

I didn't edit it knowingly, but I may have failed in copying/pasting from my terminal. I'll get better logging going, but maybe I'll just write a test.

> Are multiple threads being used here? SoupSessionAsync doesn't go to any effort
> to be threadsafe, so it's possible that that would mess things up. (In
> particular, if you called certain methods from a thread other than the thread
> whose GMainContext the session uses, then they might do bad things because they
> might queue an asynchronous operation, which could end up completing in the
> other thread even before the calling function returns.)

This may be. Have to investigate. For the time being, I have continued investigating and opened this bug report with some information I was able to gather:

http://bugzilla.gnome.org/show_bug.cgi?id=583462

Let's see if I can come up with a working (as in, failing =D) test case. 

Comment 8 Xan Lopez 2009-05-21 23:53:16 PDT
Created attachment 30572 [details]
cancelauth.patch

Apply fix suggested in GNOME's bugzilla bug, seems to work great!
Comment 9 Maciej Stachowiak 2009-05-22 04:19:17 PDT
Comment on attachment 30572 [details]
cancelauth.patch

Looks fine. I'm not sure if a regression test is possible, but if so please add one.
Comment 10 Xan Lopez 2009-05-22 12:07:22 PDT
(In reply to comment #9)
> (From update of attachment 30572 [details] [review])
> Looks fine. I'm not sure if a regression test is possible, but if so please add
> one.
> 

Thanks, landed in r44052. Writing a test for this using the webkit APIs is a bit of a pain actually, but since this is a temporary workaround until we use the fix in libsoup, and libsoup actually has a test for this, I think we should be fine.
Comment 11 Priit Laes (IRC: plaes) 2009-07-20 15:49:10 PDT
Created attachment 33117 [details]
drop-workaround.patch

We can remove the workaround now, as we depend on libsoup-2.27.4
Comment 12 Gustavo Noronha (kov) 2009-07-21 03:52:04 PDT
Comment on attachment 33117 [details]
drop-workaround.patch

Landed as r46168.