1370597 - Dismiss basic auth dialogue implicitly

Status: RESOLVED FIXED

(Testing :: geckodriver, defect)

Description

[Julius Schwartzenberg]

Attachment: patch to solve the problem

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20170203041559

Steps to reproduce:

Run a Selenium+WebDriver test that opens a URL in this form:
http://longusername@somehostname:8888/somepath


Actual results:

The test hung with a prompt asking whether I was sure I wanted to login with 'longusername' to 'somehostname'.


Expected results:

There shouldn't have been any prompt, the test should have finished properly.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Do you have any kind of further information about the pref you are using here?

Comment 2

[Julius Schwartzenberg]

There is some documentation on this preference here: http://kb.mozillazine.org/Network.http.phishy-userpass-length

The number is the maximum allowed number of characters HTTP basic auth username+password. The maximum is 255. By setting this preference, the dialog is suppressed similarly to how other security warnings are suppressed.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So none of our test harnesses is modifying this preference, and I would say that Marionette shouldn't do this neither. Usually test writers know their websites and as such can handle that dialog if in some cases the username and password are longer.

But I would leave this decision up to David or Andreas.

Comment 4

[Andreas Tolfsen ❲:ato❳]

Thanks for your patch.  It cannot be directly applied because there are some strange filenames in the patch file, but I will apply it and submit it through mozreview.

(In reply to Henrik Skupin (:whimboo) from comment #3)
> So none of our test harnesses is modifying this preference, and I would say
> that Marionette shouldn't do this neither. Usually test writers know their
> websites and as such can handle that dialog if in some cases the username
> and password are longer.

But this patch is not modifying Marionette: it is modifying geckodriver,
and WebDriver users expect this behaviour.  This is a known bug with our
WebDriver implementation.

If setting this preference causes the global modal to go away, then I’m all
for it.

Comment 5

[Julius Schwartzenberg]

Yes, that's the official Mozilla preference to suppress that dialog.

I included the path names with the Mercurial changeset ID in case it would be useful to know on which changeset I based the patch, I think it should apply with the right -p switch in any case.

In some of our tests we switch between different users within a single run, so being able to specify the username+password through the URL is very useful. It would be great if you could apply it!

Comment 6

[Andreas Tolfsen ❲:ato❳]

It would be helpful if you generated an actual .patch file instead of a diff.  This would be much easier for me to apply, but I’m fine to do the manual legwork too.

Comment 7

[Julius Schwartzenberg]

You mean a Mercurial patch? I didn't manage to figure out how to fetch just the geckodriver with Mercurial, which is why I used diff with just the geckodriver part with a tarball from the Hg web interface. I thought it would give a similar result, except that maybe the strip path option would need to be used. If there's a better trick I'd be glad to learn about it. If you can apply it manually that's great of course. Thanks!

Comment 8

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1370597 - Dismiss basic auth dialogue implicitly;

Increasing the length of the network.http.phishy-userpass-length
preference will cause Firefox to not prompt when navigating to a website
with a username or password in the URL.

Review commit: https://reviewboard.mozilla.org/r/146388/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/146388/

Comment 9

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8875031 [details]
Bug 1370597 - Dismiss basic auth dialogue implicitly;

https://reviewboard.mozilla.org/r/146388/#review150396

Comment 10

[Andreas Tolfsen ❲:ato❳]

Thanks for your patch!  I’ve rebased, reviewed it, and sent it off for integration.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/215afe84f16e
Dismiss basic auth dialogue implicitly; r=ato

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Andreas Tolfsen ‹:ato› from comment #4)
> But this patch is not modifying Marionette: it is modifying geckodriver,
> and WebDriver users expect this behaviour.  This is a known bug with our
> WebDriver implementation.

I see. In that case, lets get it in and we can ship it with the upcoming 0.16.2 release of geckodriver, which I'm currently preparing.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/215afe84f16e