Bug 107124 - [BlackBerry] Race condition clearing invalid proxy credentials in NetworkJob
Summary: [BlackBerry] Race condition clearing invalid proxy credentials in NetworkJob
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Joe Mason
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-17 07:15 PST by Joe Mason
Modified: 2013-01-18 09:39 PST (History)
5 users (show)

See Also:


Attachments
fix (5.22 KB, patch)
2013-01-17 07:23 PST, Joe Mason
yong.li.webkit: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
rebased patch (4.88 KB, patch)
2013-01-18 09:07 PST, Joe Mason
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joe Mason 2013-01-17 07:15:31 PST
NetworkJob contains this code when sending a request with proxy auth
credentials:

            // Prevent them from been used again if they are wrong.
            // If they are correct, they will be put into CredentialStorage.
            if (!proxyInfo.address.empty()) {
                proxyInfo.username.clear();
                proxyInfo.password.clear();
               
BlackBerry::Platform::Settings::instance()->storeProxyCredentials(proxyInfo);
            }

Later when notifyAuthReceived is called, the credentials used are saved in
CredentialStorage if "success" is true, or purged if "success" is false. The
intent is that from this point forward the credentials will be read from
CredentialStorage rather than Platform::Settings, and if the credentials did
not succeed they will not be in CredentialStorage so the browser will
re-prompt.

Two problems with this:

1. If the proxy server is slow to respond, another request could go out before
the success result is received, and since the credentials have already been
cleared this request would prompt the user for credentials even though the
saved credentials are correct.

2. In CredentialStorage, the credentials are indexed by auth type, and in
Settings they are not. So after successfully authenticating with, for instance,
NTLM auth, the credentials will be stored with NTLM but cleared from Settings.
If the proxy server is then reconfigured to ask for Digest auth, the browser
will prompt for credentials again rather than using the saved credentials.
Comment 1 Joe Mason 2013-01-17 07:23:40 PST
Created attachment 183182 [details]
fix
Comment 2 WebKit Review Bot 2013-01-17 12:16:59 PST
Comment on attachment 183182 [details]
fix

Rejecting attachment 183182 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 13388 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
53>At revision 13388.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/15943093
Comment 3 Joe Mason 2013-01-18 09:07:23 PST
Created attachment 183476 [details]
rebased patch

Rebased
Comment 4 WebKit Review Bot 2013-01-18 09:39:25 PST
Comment on attachment 183476 [details]
rebased patch

Clearing flags on attachment: 183476

Committed r140164: <http://trac.webkit.org/changeset/140164>
Comment 5 WebKit Review Bot 2013-01-18 09:39:28 PST
All reviewed patches have been landed.  Closing bug.