NSS should have only one function that generates error messages


17 years ago
3 years ago


(Reporter: Nelson Bolyard (seldom reads bugmail), Assigned: Nelson Bolyard (seldom reads bugmail))


Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)

NSS seems to have several functions that return error text strings 
for NSS's error codes.  

SECU_Strerror() in cmd/lib/secerror.c returns a pointer to an const
error string.  It returns strings for all known SEC_ SSL_ and NSPR
error codes.  It returns NULL for unknown error codes.  In my opinion
this function should be the ONLY source of error strings in NSS.

cmd/lib/secutil.c also contains several other functions for returning
error strings, including SECU_ErrorString(), SECU_ErrorStringRaw(),
and SECU_GetString(), and others.  These all use a comon static 
buffer to hold error messages, and so none of them is thread safe.
SECU_ErrorStringRaw provides strings for a small subset of the SEC_
and SSL_ error codes, and none for the NSPR error codes.  It's 
error messages are very terse and potentially ambiguous.
These functions are presently used in the following source files
in nss/cmd/...


I thing that SECU_ErrorStringRaw should be deleted and functions
that use it should be converted to use SEC_Strerror instead.
Also, it would be good to make the functions in secutil be 
thread safe.


17 years ago
Priority: -- → P3
Target Milestone: --- → 3.3

Comment 1

17 years ago
Assigned the bug to Kirk.

Kirk, didn't you already add such a function
(SECU_PrintPRandOSError) in bug #66244?  If
so, you just need to make sure that all the command
line utilities on Nelson's list use this new
Assignee: wtc → kirke


16 years ago
Target Milestone: 3.3 → 3.4

Comment 2

16 years ago
Changed the QA contact to Bishakha.
QA Contact: sonja.mirtitsch → bishakhabanerjee

Comment 3

16 years ago
Set target milestone to NSS 3.5.
Target Milestone: 3.4 → 3.5

Comment 4

15 years ago
Set target milestone to NSS 3.6.
Target Milestone: 3.5 → 3.6


15 years ago
Priority: P3 → P2
Target Milestone: 3.6 → 3.7

Comment 5

15 years ago
Moved to target milestone 3.8 because the original
NSS 3.7 release has been renamed 3.8.
Target Milestone: 3.7 → 3.8

Comment 6

15 years ago
Remove target milestone of 3.8, since these bugs didn't get into that release.
Target Milestone: 3.8 → ---

Comment 7

14 years ago
Target Milestone: 3.9
Target Milestone: --- → 3.9

Comment 8

14 years ago
Retargeted Future.  Plan to address Sun issues first.
Target Milestone: 3.9 → Future


14 years ago
Target Milestone: Future → ---


14 years ago
Assignee: kirk.erickson → wchang0222

Comment 9

14 years ago
The right way to fix this is to have NSS_Init install error code translations
into NSPR via PR_ErrorInstallTable, and then use the PR_ErrorToString() API for
its intended purpose.  An NSPR socket with NSS installed under it will cause
NSPR APIs to return SSL errors, so PR_ErrorToString should have the strings for
those errors installed.

This is also important because when the error to string tables are not in the
library that generates the error, then the numbers can get out of sync.  For
example, Mozilla libsslldap library has a copy of the errors and I just had to
put another copy in the async I/O layer we use for Messaging Server at Sun.  As
NSS adds new errors, these tables won't be updated automatically, so it would be
better if the tables were in libnss.so and not duplicated in everything that
calls it.

Comment 10

14 years ago
Created attachment 140280 [details]
Example of what NSPR error table for SEC errors looks like

Comment 11

14 years ago
Chris, thanks for your contribution!  

I don't think NSS_Init should install these values, but I agree the 
values should be available for NSS-based programs to use.  

Perhaps you would care to contribute a patch that shows the conversion
of a small NSS utility command, such as pk12util/pk12util.c to 
a) install error code translations into NSPR via PR_ErrorInstallTable, 
   (say, just before the call to NSS_Init) and then 
b) use the PR_ErrorToString() API for its intended purpose. 

Comment 12

14 years ago
I'm very busy at work and NSS is not my direct responsibility.  In order for me
to make time to build a full patch, I would have to get something out of it. 
The benefit I want only happens if the error tables are folded into the NSS
shared objects so I no longer have to maintain a separate error table in my
layered code  If there is agreement to do that, then I'm willing to do the
integration work as a patch (including all tools and the core libraries).  If we
don't have NSS_Init install the table, then we need to export the table itself
as a symbol so the caller can install it -- I'm fine with either approach.

Comment 13

14 years ago
Created attachment 141848 [details] [diff] [review]
alternative patch - maintenance free


This patch creates two tables, one for SEC errors and one for SSL errors.
It also includes a little function that installs those tables into NSPR's
error mechanism.  
It does NOT duplicate the contents of SECerrs.h or SSLerrs.h, but rather
includes them, so as those tables grow, there is no need to perform 
maintenance on this source file.  
Is this what you want?

If so, please contribute a small patch showing how any of the NSS 
utility commands might make use of this.  Thanks.
Assignee: wchang0222 → MisterSSL


14 years ago
Depends on: 172051

Comment 14

14 years ago
Comment on attachment 141848 [details] [diff] [review]
alternative patch - maintenance free

I have added a large patch to bug 172051 that creates a 
function that registers the NSS error strings with NSPR.
This new function becomes part of libNSS.  

I decided to include the SSL error strings in libNSS, and
have one function that registers both NSS and SSL error codes,
because NSS uses SSL error codes in places that are not 
necessarily SSL errors.  

The patch changes the common SECU_ error string functions
to (a) call the new NSS function to register its error strings,
and (b) use NSPR's set of registered error strings exclusively.

The patch does not eliminate the above named functions that
use a static error message buffer, but it does change them to
use NSPR's strings rather than their own.

After that patch is checked in and bug 172051 is closed, I can
go to work on eliminating the calls to the error functions 
that use a static error buffer, replacing them with calls to 
the safe function(s).  

I believe that the patch for bug 172051 meets Chris Newman's needs.
Attachment #141848 - Attachment is obsolete: true


12 years ago
QA Contact: bishakhabanerjee → jason.m.reid


12 years ago
QA Contact: jason.m.reid → tools


12 years ago
Blocks: 333403
You need to log in before you can comment on or make changes to this bug.