Replace PR_INTERVAL_NO_WAIT with PR_INTERVAL_NO_TIMEOUT in client initialization calls

RESOLVED FIXED in 3.12.2

Status

NSS
Libraries
P2
normal
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Alexei Volkov, Assigned: Alexei Volkov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PKIX SUN_MUST_HAVE)

Attachments

(1 attachment, 2 obsolete attachments)

41.91 KB, patch
Details | Diff | Splinter Review
(Assignee)

Description

10 years ago
PR_INTERVAL_NO_WAIT socket option makes client to use non-blocking IO. It is still untested(and apparently broken) feature, and so it should not be used until verified(or fixed).
(Assignee)

Updated

10 years ago
Priority: -- → P2
Whiteboard: PKIX
Alexei, this bug is WAY too vague.  It sounds like you're saying that 
non-blocking IO is not supported anywhere by NSS.  But in fact, 
non-blocking IO has been working for years in libSSL.  

I suspect you are referring to some specific function, or small group
of functions, but I don't know in what libraries or test programs.
So, please correct this bug to be very specific about the issue.
Version: 3.12 → trunk
After reading bug 408434 comment 8, I gather this bug is describing 
function pkix_pl_AiaMgr_FindLDAPClient in 
nss/lib/libpkix/pkix_pl_nss/module/pkix_pl_aiamgr.c .
Is there any other function that is covered by this bug?
Whiteboard: PKIX → PKIX NSS312B2
(Assignee)

Comment 3

10 years ago
> Alexei, this bug is WAY too vague.  It sounds like you're saying that 
> non-blocking IO is not supported anywhere by NSS.  But in fact, 
> non-blocking IO has been working for years in libSSL.  

This bug speaks only about libpkix non-blocking functionality.

> Is there any other function that is covered by this bug?
No. Just this one place. Bug any IO feature of libpkix(cert, crl fetching, ocsp revocation checking) can, by design, use NBIO. We just need to make sure that libpkix does not use NBIO before this libpkix feature get tested.  
(Assignee)

Updated

10 years ago
Whiteboard: PKIX NSS312B2 → PKIX
Target Milestone: 3.12 → 3.12.2
We need to think more about how timeouts are set.
(Assignee)

Comment 5

9 years ago
Retargeting to 3.12.1. Want to make sure timeouts are properly set.
Target Milestone: 3.12.2 → 3.12.1
(Assignee)

Updated

9 years ago
Target Milestone: 3.12.1 → 3.12.2
(Assignee)

Updated

9 years ago
Whiteboard: PKIX → PKIX SUN_MUST_HAVE
(Assignee)

Comment 6

9 years ago
Created attachment 343132 [details] [diff] [review]
Patch v1 use global pkix timeout

The set of changes: create global pkix structure that keeps values for pkix communication timeout and maximum transfer length. Use those values for ocsp 
responses and cert/crl transfers.
Attachment #343132 - Flags: review?(nelson)
(Assignee)

Comment 7

9 years ago
Comment on attachment 343132 [details] [diff] [review]
Patch v1 use global pkix timeout

The intention of the setter function is to change default values, but those can only use before/immediately after pkix initialization.
Attachment #343132 - Flags: review?(nelson) → review-
Comment on attachment 343132 [details] [diff] [review]
Patch v1 use global pkix timeout

Let's not define a new global, except as a _DEFAULT_ value.
Let's put the actual values used the PL Context struct.
CERT_PKIXVerifyCert should have a way for the caller to 
specify values, and if he doesn't then the defaults should 
be used.  It's OK for the defaults to be settable, but 
setting the defaults should not be the only way to change 
these values.
(Assignee)

Comment 9

9 years ago
Created attachment 343984 [details] [diff] [review]
Patch v2 - use nsscontext stored timeout

The patch also cleans up some function in pkix_pl_aiamgr.c and fixes http context leak in pkix_pl_httpcertstore.c.
Attachment #343132 - Attachment is obsolete: true
Attachment #343984 - Flags: review?(nelson)
Comment on attachment 343984 [details] [diff] [review]
Patch v2 - use nsscontext stored timeout

r+

I think maybe the setter functions below belong in a public header?

>--- lib/libpkix/pkix_pl_nss/module/pkix_pl_nsscontext.h	31 Aug 2007 03:21:38 -0000	1.3
>+++ lib/libpkix/pkix_pl_nss/module/pkix_pl_nsscontext.h	20 Oct 2008 22:42:10 -0000
>@@ -73,6 +73,12 @@ pkix_pl_NssContext_GetWincx(PKIX_PL_NssC
> PKIX_Error *
> pkix_pl_NssContext_SetWincx(void *wincx, PKIX_PL_NssContext *nssContext);
> 
>+PKIX_Error *
>+pkix_pl_NssContext_SetTimeout(PKIX_UInt32 timeout, PKIX_PL_NssContext *nssContext);
>+
>+PKIX_Error *
>+pkix_pl_NssContext_SetMaxResponseLen(PKIX_UInt32 len, PKIX_PL_NssContext *nssContext);
>+
Attachment #343984 - Flags: review?(nelson) → review+
(Assignee)

Comment 11

9 years ago
Created attachment 344694 [details] [diff] [review]
Patch v3 - integrated
Attachment #343984 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.