292156 - selfserv should allow more than 1024 SSL server session cache entries

Status: RESOLVED FIXED

(NSS :: Tools, enhancement, P2)

Description

[Julien Pierre]

selfserv should have a command-line option to configure the maximum number of
entries in the session cache, rather than using a hard-coded value of 1024 .

Comment 1

[Julien Pierre]

Attachment: add -C option

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 202595 [details] [diff] [review]
add -C option


>-#define NUM_SID_CACHE_ENTRIES 1024
>+int NUM_SID_CACHE_ENTRIES = 1024;

I think our convention is that names consisting entirely of 
capital letters and underscores are reserved for use with #defines.
I don't think we should violate that.  So, this variable name should
be changed to not look like a #define symbol.

>+        case 'C': if (optstate->value) NUM_SID_CACHE_ENTRIES = PORT_Atoi(optstate->value); break;
>+

Do we want to report an error if the option value is missing, or if
it is negative?

Comment 3

[Julien Pierre]

Attachment: check for negative maxCacheEntries and other invalid args in session cache init

Re: negative number of entries, unfortunately, libssl crashes with a SIGFPE if passed such a value. Here is a patch that fixes this problem .

Comment 4

[Julien Pierre]

Attachment: rename variable

Bug  that is filed about checking for the presence/absence of arguments for many tools, because that problem should be solved in a more generic way and is being tracked in bug 293687 .

The sign problem is taken care of by the libssl patch.

Comment 5

[Julien Pierre]

I don't know what happened to my last comment. I meant I wasn't going to check for the missing argument in the selfserv patch because there is a bug for the overall issue.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 202605 [details] [diff] [review]
check for negative maxCacheEntries and other invalid args in session cache init

r=nelson

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 202607 [details] [diff] [review]
rename variable

r=nelson

Comment 8

[Julien Pierre]

Thanks for the quick reviews, Nelson.
I checked these patches in to the tip for 3.11 .

Checking in selfserv.c;
/cvsroot/mozilla/security/nss/cmd/selfserv/selfserv.c,v  <--  selfserv.c
new revision: 1.68; previous revision: 1.67
done

Checking in sslsnce.c;
/cvsroot/mozilla/security/nss/lib/ssl/sslsnce.c,v  <--  sslsnce.c
new revision: 1.36; previous revision: 1.35
done