RESOLVED FIXED 24818
[GTK] http auth dialog pops up twice after a cancelled atempt
https://bugs.webkit.org/show_bug.cgi?id=24818
Summary [GTK] http auth dialog pops up twice after a cancelled atempt
Gustavo Noronha (kov)
Reported 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.
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
cancelauth.patch (2.42 KB, patch)
2009-05-21 23:53 PDT, Xan Lopez
mjs: review+
drop-workaround.patch (2.08 KB, patch)
2009-07-20 15:49 PDT, Priit Laes (IRC: plaes)
zecke: review+
Xan Lopez
Comment 1 2009-03-28 14:01:12 PDT
I guess we should we doing something in Cancel, but not sure what. CCing Dan.
Dan Winship
Comment 2 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.
Dan Winship
Comment 3 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?
Gustavo Noronha (kov)
Comment 4 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.
Gustavo Noronha (kov)
Comment 5 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.
Dan Winship
Comment 6 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.)
Gustavo Noronha (kov)
Comment 7 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.
Xan Lopez
Comment 8 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!
Maciej Stachowiak
Comment 9 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.
Xan Lopez
Comment 10 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.
Priit Laes (IRC: plaes)
Comment 11 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
Gustavo Noronha (kov)
Comment 12 2009-07-21 03:52:04 PDT
Comment on attachment 33117 [details] drop-workaround.patch Landed as r46168.
Note You need to log in before you can comment on or make changes to this bug.