1312062 - Port bug 1264562 to c-c [Add firstPartyDomain to socket transport ]

Status: RESOLVED FIXED

(MailNews Core :: LDAP Integration, defect)

Description

[Richard Marti (:Paenglab)]

An actual build fails with ldap/xpcom/src/nsLDAPSecurityGlue.cpp(178): error C2660: 'nsISocketProvider::AddToSocket': function does not take 7 arguments

Comment 1

[:aceman]

Attachment: WIP patch

I don't know if this is correct, but at least makes the build work.

Comment 2

[Jorg K (CEST = GMT+2)]

Comment on attachment 8803513 [details] [diff] [review]
WIP patch

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

Looks good to me looking at
https://hg.mozilla.org/mozilla-central/rev/6ed882bc57d4

Comment 3

[:aceman]

Yes, that was my template :) But do we know if we can ignore the new argument (we do not use the feature), or if we need to set it to something useful?

Comment 4

[Jorg K (CEST = GMT+2)]

     /**
+     * This is only non-empty when "privacy.firstparty.isolate" is enabled.
+     * It is used to create sockets, and will eventually be used to isolate
+     * OCSP cache. It's the only way to carry it down to NSPR layers which are
+     * final consumers.  It must be set before the socket transport is built.
+     */
+    attribute AUTF8String firstPartyDomain;

We know nothing about privacy.firstparty.isolate, so the string must be empty.

Comment 5

[aleth [:aleth]]

(In reply to :aceman from comment #3)
> Yes, that was my template :) But do we know if we can ignore the new
> argument (we do not use the feature), or if we need to set it to something
> useful?

Just request feedback from those driving the original bug. Currently it looks like the whole thing is behind a pref, but as it's a security bug it might be good to know more.

Comment 6

[aleth [:aleth]]

No reason not to land this meanwhile of course ;)

Comment 7

[:aceman]

Do you guys have an idea if we need to use this argument in Thunderbird for any purpose? Or is it only intended to be used for OCSP?

Comment 8

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

It's for first-party isolation (see bug 1299996). If the feature is enabled, you might pass some sort of identifier that ensures the desired properties of the feature, but it's probably safe to just pass an empty string in this case.

Comment 9

[Jorg K (CEST = GMT+2)]

(In reply to aleth [:aleth] from comment #6)
> No reason not to land this meanwhile of course ;)

Landed:
https://hg.mozilla.org/comm-central/rev/46b456a3059e10c10576bed1a97e533f727247ce
(Tree was busted "twice" (two bustages), needed to get ready for Daily build).

If despite comment #8 the patch isn't right, we can fix it later.

Comment 10

[Honza Bambas (:mayhemer)]

The first party is valid only when you are navigating pages in a tabbed browser.  For LDAP I think you are ok with an empty string, as you did ;)

Comment 11

[Philip Chee]

c:/t1/hg/comm-central/ldap/xpcom/src/nsLDAPSecurityGlue.cpp(180): error C2664: 
'nsresult nsISocketProvider::AddToSocket(int32_t,const char 
*,int32_t,nsIProxyInfo *,const mozilla::NeckoOriginAttributes 
&,uint32_t,PRFileDesc *,nsISupports **)': cannot convert argument 5 from 
'const nsAFlatCString' to 'const mozilla::NeckoOriginAttributes &'

c:/t1/hg/comm-central/ldap/xpcom/src/nsLDAPSecurityGlue.cpp(180): note: 
Reason: cannot convert from 'const nsAFlatCString' to 'const 
mozilla::NeckoOriginAttributes'

c:/t1/hg/comm-central/ldap/xpcom/src/nsLDAPSecurityGlue.cpp(180): note: No 
user-defined-conversion operator available that can perform this conversion, 
or the operator cannot be called

c:/t1/hg/comm-central/mozilla/config/rules.mk:963: recipe for target 
'nsLDAPSecurityGlue.obj' failed

Comment 12

[Jorg K (CEST = GMT+2)]

Look like I just fixed that.
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=84b356fa5bdcd06873d297a689452e32dcee3d6e
https://hg.mozilla.org/comm-central/rev/84b356fa5bdcd06873d297a689452e32dcee3d6e

Comment 13

[Jorg K (CEST = GMT+2)]

s/Look/Looks/ ;-(

Comment 14

[:aceman]

I'm not sure why this was pasted into this bug, it seems to be a new bustage.
Anyway, thanks Jorg.

Comment 15

[Philip Chee]

(In reply to :aceman from comment #14)
> I'm not sure why this was pasted into this bug, it seems to be a new bustage.
You were the last person to touch this file. Ergo you.