1037082 - HTTPS proxying proxy auth (407) problems

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: part 1/3 dont doauthretry with closed persistent connection

Comment 2

[Patrick McManus [:mcmanus]]

Attachment: part 2/3 407 proxy auth for http over https

Comment 3

[Patrick McManus [:mcmanus]]

Attachment: part 3/3 407 proxy auth for https over https

Comment 4

[Patrick McManus [:mcmanus]]

https://tbpl.mozilla.org/?tree=Try&rev=2fa3b4401cb3

Comment 5

[u408661]

Comment on attachment 8453950 [details] [diff] [review]
part 3/3 407 proxy auth for https over https

Review of attachment 8453950 [details] [diff] [review]:
-----------------------------------------------------------------

Side note: this looks like it will also fix an issue I noticed (but had not yet filed) while debugging bug 1014600 (namely that the squid error I get back when trying to connect to a host that doesn't answer never shows up). So, yay!

Comment 6

[Patrick McManus [:mcmanus]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/01ccb9eab0f9
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/697a57c606c4
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/cb0ebf1d80ec

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/01ccb9eab0f9
https://hg.mozilla.org/mozilla-central/rev/697a57c606c4
https://hg.mozilla.org/mozilla-central/rev/cb0ebf1d80ec

Comment 8

[Patrick McManus [:mcmanus]]

Attachment: gecko 32 version of patch

Approval Request Comment
[Feature/regressing bug #]:378637
[User impact if declined]: New feature does not do proxy auth correctly
[Describe test coverage new/current, TBPL]: on m-c, hand tested to same level as non https proxying
[Risks and why]: risks are constrained to new feature which is on aurora - feature has broken corner case without patch
[String/UUID change made/needed]: none

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Although this only landed on m-c yesterday, I'm not sure that giving this another day is going to buy us anything in terms of additional testing. I don't see any tests in the patch. Are there existing automated tests for this functionality?

Comment 10

[Patrick McManus [:mcmanus]]

unfortunately we don't have any automated proxy tests in our CI at all - and this feature is an extension of that capability, so it had to be tested by hand right now. Building something is on my todo list, but its considerably bigger than this fix, or even the feature it fixes.

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8456337 [details] [diff] [review]
gecko 32 version of patch

Aurora+

Patrick - Understandable about automating tests for this feature. Can you please run your manual sanity checks on Aurora once this lands?

Comment 12

[Patrick McManus [:mcmanus]]

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/abc10706ecbb

Comment 13

[Patrick McManus [:mcmanus]]

(In reply to Lawrence Mandel [:lmandel] from comment #11)
> . Can you
> please run your manual sanity checks on Aurora once this lands?

yes - I will. I've also already manually checked the patch on a local build of the aurora tree.

Comment 14

[Patrick McManus [:mcmanus]]

(In reply to Lawrence Mandel [:lmandel] from comment #11)
 Can you
> please run your manual sanity checks on Aurora once this lands?

done