166396 - embedders should be able to override the default socket type used for HTTP connections

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect, P2)

Description

[Darin Fisher]

embedders should be able to override the default socket type used for HTTP
connections.  we should provide a preference for this.

Comment 1

[Darin Fisher]

Attachment: v1 patch

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 97611 [details] [diff] [review]
v1 patch


Do you have any fallback plans?  For example, suppose that a pref pointing to a
socket transport provider is left but the provider is removed from disk.

Also, what about overriding the default http SSL socket?

Comment 3

[Darin Fisher]

good point... this could definitely could be expanded upon, and a sanity check
that a nsISocketProvider exists corresponding to the preference value is
probably needed.  ok, i'll rev the patch.  as for SSL, i think we should prefer
our builtin code for that.  i don't think we want to support an external SSL
socket implementation just yet.  anyways, it could always use XPCOM to make
itself the nsISocketProvider implementation of ssl.  this pref is about using a
socket type when none would normally be used.

Comment 4

[Darin Fisher]

Attachment: v2 patch

same patch, but now we try to instantiate the specified socket provider to
verify that the value provided in the preferences actually works.  if not, then
we fallback on the default setting (which is NULL).

Comment 5

[Doug Turner (:dougt)]

Comment on attachment 97708 [details] [diff] [review]
v2 patch

what about the ssl override?


     if (mConnectionInfo->UsingSSL())
	 ...GetDefaultSSLType...

also, what if the pref is set but it is empty.	You should do something like:
+	 if (NS_SUCCEEDED(rv) && val)

Comment 6

[Darin Fisher]

can preferences really return NS_OK with a NULL result?  despite that, your
comment made me think of another problem... if someone wanted to reset the
preference to its default value, they'd want to call SetCharPref(..., "") with
an empty string value.  but, the pref observer code would fail to modify
mDefaultSocketType on account of the fact that "" is not a valid socket type. 
oops!  new patch coming up ;-)

Comment 7

[Darin Fisher]

Attachment: v3 patch

Comment 8

[Darin Fisher]

Comment on attachment 97785 [details] [diff] [review]
v3 patch

got an r=dougt over AIM.

Comment 9

[rpotts (gone)]

Comment on attachment 97785 [details] [diff] [review]
v3 patch

sr=[e-mail]

Comment 10

[chris hofmann]

Comment on attachment 97785 [details] [diff] [review]
v3 patch

a=chofmann for the trunk

Comment 11

[Darin Fisher]

fixed-on-trunk (for mozilla 1.2 alpha)

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

a=[e-mail] for 1.0 branch.  Change mozilla1.0.2+ to fixed1.0.2 when
checked in 

Comment 13

[Tom Everingham]

verified - checked with lxr that patch was applied

Comment 14

[Gayatri Rimola]

Please verify the bug. Once verified, change the keyword fixed1.0.2 to
verified1.0.2 

Comment 15

[lchiang]

Tom, can you do the same verification within LXR for the mozilla 1.0 branch?

Comment 16

[Tom Everingham]

yes, verified patch applied to 1.0 branch