390187 - PK11_FindCertFromNickname sets no error code when token not found

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

When PK11_FindCertFromNickname gets a nickname with a colon in it, it calls
NSSTrustDomain_FindTokenByName to find a token with the given name.  
That function is a "Stan" function. Stan functions do not call PORT_SetError.
So, if that function returns null, no error code is set.  
Consequently, some random previously-set error code is displayed 
NSSTrustDomain_FindTokenByName should set an error in this case.
The error should signify that no token was found by the desired name.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

Should do the job

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

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

This patch is mutually exclusive with the patch for bug 390188.  
One of these bugs should be marked WONTFIX.
Please vote for one.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v2 - fix both functions

This patch fixes both functions in the file that have the same problem.
This patch is mutually exclusive with the one attached to bug 390188.
Please vote for one.

Comment 4

[Robert Relyea]

Comment on attachment 274560 [details] [diff] [review]
patch v2 - fix both functions

r+ The ':' semantic has been in NSS a long time, With this patch, the users will get a less confusing error message if they mistype the token name.

Note: There is an element of the patch for bug 390188 that can still be picked up.... The colapsing of the error handling at the end of the function.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

My own preference would be for the patch in bug 390188.  
Having a colon in a friendly name in a PKCS#12 file seems to occur
fairly frequently. But there is a workaround, which is to prefix the 
nickname with "NSS Certificate DB:"

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Fixed on trunk
Checking in pk11cert.c; new revision: 1.157; previous revision: 1.156

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 274560 [details] [diff] [review]
patch v2 - fix both functions

Neil, please review this patch for the branch.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 274560 [details] [diff] [review]
patch v2 - fix both functions

Julien, since you wrote in bug 390188 that you'd prefer this patch, please review it.

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

Thanks for the reviews
Checking in pk11wrap/pk11cert.c; new: 1.143.2.10; previous: 1.143.2.9

Comment 10

[Neil Williams]

(In reply to comment #7)
> (From update of attachment 274560 [details] [diff] [review])
> Neil, please review this patch for the branch.
> 

Somehow my review request disappeared. Anyhow, r+.