322998 - SSLServerSocket.accept() consumes the cause of exception. It needs to propagate the exception

Status: RESOLVED FIXED

(JSS Graveyard :: Library, defect)

Description

[Sandeep Konchady]

In org/mozilla/jss/ssl/SSLServerSocket.java accept() method consumes the cause of exception and does not propagate it. So if there is an exception and accept fails, the cause is unknown. At the least accept needs to propagate the message from the exception.

                ....
                ....
                s.setSockProxy(sp);
            } catch (SocketTimeoutException e) {
                /* unnessary to do a s.close() since exception thrown*/
                throw new SocketTimeoutException
                        ("server socket accept timed out");
            } catch (Exception e) {
                /* unnessary to do a s.close() since exception thrown*/
                throw new IOException("accept method failed");
            } finally {
                ....
                ....

This needs to be fixed in JSS 3.1.x, 4.1.x as well as 4.2.x branches.

Comment 1

[Sandeep Konchady]

Attachment: Modification to SSLServerSocket on JSS 3.1.x branch to propagate the native exceptions

This patch is for JSS 3.1.10. Currently SSLServerSocket.java is catching all exceptions and converting them into IOException. Application using the older versions of JSS were seeing SSLSocketException that was being thrown from SSLServerSocket.c. Due to the conversion of this SSLSocketException to IOException the applications are having crashes or hangs.

To address this I am removing the catch(Exception e) block from the accept() method. This way, what ever exception is caught and thrown from native call is what will be propagated. Currently JSS 3.1.x is only throwing IOException and SocketException. SocketException is a inherited from IOException, so if someone is not already catching SocketException, it is ok.

Comment 2

[Alexei Volkov]

Comment on attachment 208304 [details] [diff] [review]
Modification to SSLServerSocket on JSS 3.1.x branch to propagate the native exceptions

*No need to add SocketException to the list of exceptions that
accept throws, since SocketException derives from IOException.

*We know that GW ignores all SocketException and restart SSLServerSocket
upon IOExeption. Now the patch swings the behavior of SSLServerSocket.accept function in the other way: instead of throwing IOException all the time,
we will through SocketException all the time. It means that all our exceptions will be ignored, we potentially create possibilities for bugs and escalation.

I think we need to identify what NSS/NSPR errors can/cannot be ignored and
throw appropriate exceptions.

Comment 3

[Sandeep Konchady]

Attachment: Same as before but with addition to common.c

As per Alexei's request, I did the following modifications for JSS 3.1.x:

[1] SSLServerSocket.java does not throw SocketException any more.
[2] In common.c I added check for PR_IO_ERROR in JSSL_throwSSLSocketException. This now allows for this method to throw INTERRUPTED_IO_EXCEPTION, IO_EXCEPTION and SOCKET_EXCEPTION.

Comment 4

[Alexei Volkov]

Comment on attachment 208399 [details] [diff] [review]
Same as before but with addition to common.c

The fix looks like a solution for the patch for 3.1.10 version. But for future we need to do better job separating IO for Socket exceptions

Comment 5

[glen beasley]

Comment on attachment 208399 [details] [diff] [review]
Same as before but with addition to common.c

approved for 3.1 branch.

Comment 6

[Sandeep Konchady]

Checking in the code for JSS 3.1.11.

Checking in org/mozilla/jss/manage/CryptoManager.c;
/cvsroot/mozilla/security/jss/org/mozilla/jss/manage/Attic/CryptoManager.c,v  <--  CryptoManager.c
new revision: 1.11.2.2.2.8.2.19; previous revision: 1.11.2.2.2.8.2.18
done
Checking in org/mozilla/jss/manage/CryptoManager.java;
/cvsroot/mozilla/security/jss/org/mozilla/jss/manage/Attic/CryptoManager.java,v  <--  CryptoManager.java
new revision: 1.7.2.3.2.12.2.17; previous revision: 1.7.2.3.2.12.2.16
done
Checking in org/mozilla/jss/ssl/SSLServerSocket.c;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/SSLServerSocket.c,v  <--  SSLServerSocket.c
new revision: 1.4.4.2.2.2.2.2; previous revision: 1.4.4.2.2.2.2.1
done
Checking in org/mozilla/jss/ssl/SSLServerSocket.java;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/SSLServerSocket.java,v  <--  SSLServerSocket.java
new revision: 1.5.4.1.2.3.2.6; previous revision: 1.5.4.1.2.3.2.5
done
Checking in org/mozilla/jss/ssl/common.c;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/common.c,v  <--  common.c
new revision: 1.6.8.1.2.1.2.9; previous revision: 1.6.8.1.2.1.2.8
done
Checking in org/mozilla/jss/util/jss_exceptions.h;
/cvsroot/mozilla/security/jss/org/mozilla/jss/util/jss_exceptions.h,v  <--  jss_exceptions.h
new revision: 1.3.6.1.2.1; previous revision: 1.3.6.1
done
Checking in org/mozilla/jss/util/jssver.h;
/cvsroot/mozilla/security/jss/org/mozilla/jss/util/jssver.h,v  <--  jssver.h
new revision: 1.1.2.4.2.3.2.16; previous revision: 1.1.2.4.2.3.2.15
done

Comment 7

[Sandeep Konchady]

Looking at the possible exceptions that are throwable as per JDK 1.4.2 javadoc for SSLServerSocket.accept(), it looks like we are only throwing two. Javadoc has the following exceptions :

IOException - if an I/O error occurs when waiting for a connection. 
SecurityException - if a security manager exists and its checkListen method 
                    doesn't allow the operation. 
SocketTimeoutException - if a timeout was previously set with setSoTimeout and 
                         the timeout has been reached. 
IllegalBlockingModeException - if this socket has an associated channel, and the
                               channel is in non-blocking mode.

My proposal would be for us to appropriately catch PR_ERROR codes and convert them into Java exceptions that are allowable by accept(). Attached below is the list of PR_ERROR and an approximation of the Java exceptions that these come under. This list needs to be reviewed and once approved will be implemented in JSS 4.1.x as well as 4.2.x

[1] java.lang.OutOfMemoryException
Cannot throw this exception as the only exceptions that are 
throwable are the ones that are derived from java.io.IOException.
So placing this under java.net.SocketException
/* Memory allocation attempt failed */
#define PR_OUT_OF_MEMORY_ERROR                   (-6000L)

[2] java.net.SocketException
/* Invalid file descriptor */
#define PR_BAD_DESCRIPTOR_ERROR                  (-5999L)
/* The operation would have blocked */
#define PR_WOULD_BLOCK_ERROR                     (-5998L)
/* Invalid memory address argument */
#define PR_ACCESS_FAULT_ERROR                    (-5997L)
/* Invalid function for file type */
#define PR_INVALID_METHOD_ERROR                  (-5996L)
/* Invalid memory address argument */
#define PR_ILLEGAL_ACCESS_ERROR                  (-5995L)
/* function not implemented */
#define PR_NOT_IMPLEMENTED_ERROR                 (-5992L)
/* I/O operation on busy file descriptor */
#define PR_IO_PENDING_ERROR                      (-5989L)
/* The directory could not be opened */
#define PR_DIRECTORY_OPEN_ERROR                  (-5988L)
/* Invalid function argument */
#define PR_INVALID_ARGUMENT_ERROR                (-5987L)
/* Network file descriptor is not connected */
#define PR_NOT_CONNECTED_ERROR                   (-5978L)
/* Failure to load dynamic library */
#define PR_LOAD_LIBRARY_ERROR                    (-5977L)
/* Failure to unload dynamic library */
#define PR_UNLOAD_LIBRARY_ERROR                  (-5976L)
/* Symbol not found in any of the loaded dynamic libraries */
#define PR_FIND_SYMBOL_ERROR                     (-5975L)
/* Insufficient system resources */
#define PR_INSUFFICIENT_RESOURCES_ERROR          (-5974L)
/* A directory lookup on a network address has failed */
#define PR_DIRECTORY_LOOKUP_ERROR                (-5973L)
/* Attempt to access a TPD key that is out of range */
#define PR_TPD_RANGE_ERROR                       (-5972L)
/* Process open FD table is full */
#define PR_PROC_DESC_TABLE_FULL_ERROR            (-5971L)
/* System open FD table is full */
#define PR_SYS_DESC_TABLE_FULL_ERROR             (-5970L)
/* Network operation attempted on non-network file descriptor */
#define PR_NOT_SOCKET_ERROR                      (-5969L)
/* TCP-specific function attempted on a non-TCP file descriptor */
#define PR_NOT_TCP_SOCKET_ERROR                  (-5968L)
/* TCP file descriptor is already bound */
#define PR_SOCKET_ADDRESS_IS_BOUND_ERROR         (-5967L)
/* Access Denied */
#define PR_NO_ACCESS_RIGHTS_ERROR                (-5966L)
/* Access to the remote file has been severed */
#define PR_REMOTE_FILE_ERROR                     (-5963L)
/* The value requested is too large to be stored in the data buffer provided */
#define PR_BUFFER_OVERFLOW_ERROR                 (-5962L)
/* TCP connection reset by peer */
#define PR_CONNECT_RESET_ERROR                   (-5961L)
/* Unused */
#define PR_RANGE_ERROR                           (-5960L)
/* The operation would have deadlocked */
#define PR_DEADLOCK_ERROR                        (-5959L)
/* The file is already locked */
#define PR_FILE_IS_LOCKED_ERROR                  (-5958L)
/* Write would result in file larger than the system allows */
#define PR_FILE_TOO_BIG_ERROR                    (-5957L)
/* The device for storing the file is full */
#define PR_NO_DEVICE_SPACE_ERROR                 (-5956L)
/* Unused */
#define PR_PIPE_ERROR                            (-5955L)
/* Unused */
#define PR_NO_SEEK_DEVICE_ERROR                  (-5954L)
/* Cannot perform a normal file operation on a directory */
#define PR_IS_DIRECTORY_ERROR                    (-5953L)
/* Symbolic link loop */
#define PR_LOOP_ERROR                            (-5952L)
/* File name is too long */
#define PR_NAME_TOO_LONG_ERROR                   (-5951L)
/* Cannot perform directory operation on a normal file */
#define PR_NOT_DIRECTORY_ERROR                   (-5949L)
/* Cannot write to a read-only file system */
#define PR_READ_ONLY_FILESYSTEM_ERROR            (-5948L)
/* Cannot delete a directory that is not empty */
#define PR_DIRECTORY_NOT_EMPTY_ERROR             (-5947L)
/* Cannot delete or rename a file object while the file system is busy */
#define PR_FILESYSTEM_MOUNTED_ERROR              (-5946L)
/* Cannot rename a file to a file system on another device */
#define PR_NOT_SAME_DEVICE_ERROR                 (-5945L)
/* The directory object in the file system is corrupted */
#define PR_DIRECTORY_CORRUPTED_ERROR             (-5944L)
/* Cannot create or rename a filename that already exists */
#define PR_FILE_EXISTS_ERROR                     (-5943L)
/* Directory is full.  No additional filenames may be added */
#define PR_MAX_DIRECTORY_ENTRIES_ERROR           (-5942L)
/* The required device was in an invalid state */
#define PR_INVALID_DEVICE_STATE_ERROR            (-5941L)
/* The device is locked */
#define PR_DEVICE_IS_LOCKED_ERROR                (-5940L)
/* No more entries in the directory */
#define PR_NO_MORE_FILES_ERROR                   (-5939L)
/* Seek error */
#define PR_FILE_SEEK_ERROR                       (-5937L)
/* The file is busy */
#define PR_FILE_IS_BUSY_ERROR                    (-5936L)
/* Operation is still in progress (probably a non-blocking connect) */
#define PR_IN_PROGRESS_ERROR                     (-5934L)
/* Operation has already been initiated (probably a non-blocking connect) */
#define PR_ALREADY_INITIATED_ERROR               (-5933L)
/* The wait group is empty */
#define PR_GROUP_EMPTY_ERROR                     (-5932L)
/* Object state improper for request */
#define PR_INVALID_STATE_ERROR                   (-5931L)
/* Network is down */
#define PR_NETWORK_DOWN_ERROR                    (-5930L)
/* Socket shutdown */
#define PR_SOCKET_SHUTDOWN_ERROR                 (-5929L)
/* The library is not loaded */
#define PR_LIBRARY_NOT_LOADED_ERROR              (-5926L)
/* Some unknown error has occurred */
#define PR_UNKNOWN_ERROR                         (-5994L)

[3] java.io.InterruptedIOException
/* Operation interrupted by another thread */
#define PR_PENDING_INTERRUPT_ERROR               (-5993L)
/* The I/O operation was aborted */
#define PR_OPERATION_ABORTED_ERROR               (-5935L)

[4] java.io.IOException
/* I/O function error */
#define PR_IO_ERROR                              (-5991L)

[5] java.net.SocketTimeoutException
/* I/O operation timed out */
#define PR_IO_TIMEOUT_ERROR                      (-5990L)
/* Connection attempt timed out */
#define PR_CONNECT_TIMEOUT_ERROR                 (-5979L)

[6] java.net.ConnectException
/* Network address not available (in use?) */
#define PR_ADDRESS_NOT_AVAILABLE_ERROR           (-5986L)
/* Network address type not supported */
#define PR_ADDRESS_NOT_SUPPORTED_ERROR           (-5985L)
/* Already connected */
#define PR_IS_CONNECTED_ERROR                    (-5984L)
/* Local Network address is in use */
#define PR_ADDRESS_IN_USE_ERROR                  (-5982L)
/* Connection refused by peer */
#define PR_CONNECT_REFUSED_ERROR                 (-5981L)
/* Connection aborted */
#define PR_CONNECT_ABORTED_ERROR                 (-5928L)

[7] java.net.MalformedURLException
/* Network address is invalid */
#define PR_BAD_ADDRESS_ERROR                     (-5983L)

[8] java.net.UnknownHostException
/* Network address is presently unreachable */
#define PR_NETWORK_UNREACHABLE_ERROR             (-5980L)

[9] java.net.UnknownServiceException
/* The requested operation is not supported by the platform */
#define PR_OPERATION_NOT_SUPPORTED_ERROR         (-5965L)

[10] java.net.ProtocolException
/* The host operating system does not support the protocol requested */
#define PR_PROTOCOL_NOT_SUPPORTED_ERROR          (-5964L)

[11] java.io.FileNotFoundException
/* File not found */
#define PR_FILE_NOT_FOUND_ERROR                  (-5950L)

[12] java.io.EOFException
/* Encountered end of file */
#define PR_END_OF_FILE_ERROR                     (-5938L)

[13] java.net.NoRouteToHostException
/* Host is unreachable */
#define PR_HOST_UNREACHABLE_ERROR                (-5927L)

Comment 8

[Sandeep Konchady]

Wan-Teh's comments

I quickly went through your document.  Many of
the mappings are good.  Some I am not sure.
A few are clearly wrong, for example:

> [7] java.net.MalformedURLException
> /* Network address is invalid */
> #define PR_BAD_ADDRESS_ERROR                     (-5983L)


NSPR doesn't handle URLs!

Wan-Teh 

Comment 9

[Sandeep Konchady]

Based on Wan-Teh and Alexei's inputs, these are the changes to the mappings. All exceptions under 
java.net.MalformedURLException, 
java.net.UnknownHostException, 
java.net.UnknownServiceException, 
java.net.ProtocolException,
java.net.NoRouteToHostException 
have been mapped to java.net.ConnectException. If this looks reasonable I will modify the code to map to these PR_ERRORs.

153,154d152
<
< [7] java.net.MalformedURLException
157,158d154
<
< [8] java.net.UnknownHostException
161,162d156
<
< [9] java.net.UnknownServiceException
165,166d158
<
< [10] java.net.ProtocolException
168a161,162
> /* Host is unreachable */
> #define PR_HOST_UNREACHABLE_ERROR                (-5927L)
170c164
< [11] java.io.FileNotFoundException
---
> [7] java.io.FileNotFoundException
174c168
< [12] java.io.EOFException
---
> [8] java.io.EOFException
177,180d170
<
< [13] java.net.NoRouteToHostException
< /* Host is unreachable */
< #define PR_HOST_UNREACHABLE_ERROR                (-5927L)

Comment 10

[Sandeep Konchady]

Attachment: Added Java exceptions corresponding the PR_ERROR codes as per the discussion in this bug.

Added Java exceptions corresponding the PR_ERROR codes as per the discussion in this bug.

Comment 11

[Alexei Volkov]

Comment on attachment 210158 [details] [diff] [review]
Added Java exceptions corresponding the PR_ERROR codes as per the discussion in this bug.

Here are the comments:

* SSLServerSocket.c: remove printf. Everything get delivered through exception s.

* Would be nice to have at least some message(javadoc compliant) before SSLServerSocket::accept method that will describe what exception are been thrown.

* common.c: lots of code dup code in each if statement. Similar code should be preferably used in one place only. Also, I would use "switch" instead of "if".

* jss_exceptions.c: please put exceptions in alphabetical order.

Comment 12

[Sandeep Konchady]

Attachment: Modified the contents as per Alexei's suggestion

Modified the sources as per Alexei's suggestion.

Comment 13

[Alexei Volkov]

Comment on attachment 210287 [details] [diff] [review]
Modified the contents as per Alexei's suggestion

Looks OK except:
 * in common.c throw exception function may issue a warning if for some reason the exception was not thrown.

 * missing description of
return object in comments part of SSLServerSocket:accept. Understand that this is obvious, but lets make it done properely

  * I beliave exceptions FileNotFoundException, EOFException are nether thrown since NSPR can not generate these error codes during accept procedure.

Comment 14

[Wan-Teh Chang]

Comment on attachment 210287 [details] [diff] [review]
Modified the contents as per Alexei's suggestion

1. org/mozilla/jss/ssl/SSLServerSocket.c

>+        } else if( err == PR_IO_ERROR ) {
>+#ifdef WINNT
>+            PR_NT_CancelIo(sock->fd);
>+#endif
>+            JSSL_throwSSLSocketException(env, "Accept received IO error");

Remove the PR_NT_CancelIo call.  We only need to call
PR_NT_CancelIo for two errors:
PR_PENDING_INTERRUPT_ERROR and PR_IO_TIMEOUT_ERROR.

2. org/mozilla/jss/ssl/SSLServerSocket.java

>     /**
>      * Accepts a connection. This call will block until a connection is made
>-     *   or the timeout is reached.
>+     * or the timeout is reached.
>+     * @throws IOException  If an input or output exception occurred
>+     * @throws SocketTimeoutException  If the socket timesout tryinng to connect
>+     * @throws InterruptedIOException  If an input or output is interrupted
>+     * @throws ConnectException  If there is an error connecting to resource
>+     * @throws FileNotFoundException  If file is not found
>+     * @throws EOFException  If End Of File is reached
>      */
>     public Socket accept() throws IOException,SocketTimeoutException {

Why don't you need to list InterruptedIOException,
ConnectException, FileNotFoundException, and
EOFException in the method's prototype?

FileNotFoundException doesn't make sense for accept().
EOFException may not make sense for accept(), either.

The comment after ConnectException doesn't make sense
for accept().  I'm not sure if ConnectException itself
makes sense for accept(), either.

3. org/mozilla/jss/ssl/common.c

The new code using the switch statement is much
clearer.

>+        case PR_ADDRESS_NOT_AVAILABLE_ERROR :
>+        case PR_ADDRESS_NOT_SUPPORTED_ERROR :
>+        case PR_IS_CONNECTED_ERROR :
>+        case PR_ADDRESS_IN_USE_ERROR :
>+        case PR_CONNECT_REFUSED_ERROR :
>+        case PR_CONNECT_ABORTED_ERROR :
>+        case PR_BAD_ADDRESS_ERROR :
>+        case PR_NETWORK_UNREACHABLE_ERROR :
>+        case PR_OPERATION_NOT_SUPPORTED_ERROR :
>+        case PR_PROTOCOL_NOT_SUPPORTED_ERROR :
>+        case PR_HOST_UNREACHABLE_ERROR :
>+            excepClass = (*env)->FindClass(env, CONNECT_EXCEPTION);
>+            break;
>+        case PR_PENDING_INTERRUPT_ERROR :
>+        case PR_OPERATION_ABORTED_ERROR :
>+            excepClass = (*env)->FindClass(env, INTERRUPTED_IO_EXCEPTION);
>+            break;

I have to admit that I'm not sure about the mapping
of some of the above errors:
PR_ADDRESS_NOT_AVAILABLE_ERROR
PR_ADDRESS_NOT_SUPPORTED_ERROR
PR_BAD_ADDRESS_ERROR
PR_OPERATION_NOT_SUPPORTED_ERROR
PR_PROTOCOL_NOT_SUPPORTED_ERROR
PR_OPERATION_ABORTED_ERROR

Some of these may be returned by other socket I/O
functions than connect().  Some of these I don't
know what they mean exactly.

4. org/mozilla/jss/util/jss_exceptions.h

>+#define FILENOTFOUND_EXCEPTION "java/io/FileNotFoundException"

Nit: I suggest renaming this FILE_NOT_FOUND_EXCEPTION.

Comment 15

[Sandeep Konchady]

Attachment: Modifications based on Alexei and Wan-Teh's feedback

------- Comment #13 from [e-mail]  2006-02-01 10:19 PST -----
(From update of attachment 210287 [details] [diff] [review])
Looks OK except:
 * missing description of
return object in comments part of SSLServerSocket:accept(). Understand that this
is obvious, but lets make it done properely

Sandeep: Added @return to javadoc

  * I beliave exceptions FileNotFoundException, EOFException are nether thrown
since NSPR can not generate these error codes during accept procedure.

Sandeep: Removed references to FileNotFoundException and EOFException.

------- Comment #14 from [e-mail]  2006-02-02 17:20 PST -------
(From update of attachment 210287 [details] [diff] [review])
1. org/mozilla/jss/ssl/SSLServerSocket.c


>>+        } else if( err == PR_IO_ERROR ) {
>>+#ifdef WINNT
>>+            PR_NT_CancelIo(sock->fd);
>>+#endif
>>+            JSSL_throwSSLSocketException(env, "Accept received IO error");


Remove the PR_NT_CancelIo call.  We only need to call
PR_NT_CancelIo for two errors:
PR_PENDING_INTERRUPT_ERROR and PR_IO_TIMEOUT_ERROR.

Sandeep: Done

2. org/mozilla/jss/ssl/SSLServerSocket.java

Why don't you need to list InterruptedIOException,
ConnectException, FileNotFoundException, and
EOFException in the method's prototype?

FileNotFoundException doesn't make sense for accept().
EOFException may not make sense for accept(), either.

The comment after ConnectException doesn't make sense
for accept().  I'm not sure if ConnectException itself
makes sense for accept(), either.

Sandeep: Removed references to FileNotFoundException, EOFException and ConnectException. Also the prototype as per J2SE java.net.ServerSocket.accept() only throws IOException. All other exceptions are derived from IOException and need not be listed in the prototype, but documented in javadoc.

3. org/mozilla/jss/ssl/common.c

I have to admit that I'm not sure about the mapping
of some of the above errors:
PR_ADDRESS_NOT_AVAILABLE_ERROR
PR_ADDRESS_NOT_SUPPORTED_ERROR
PR_BAD_ADDRESS_ERROR
PR_OPERATION_NOT_SUPPORTED_ERROR
PR_PROTOCOL_NOT_SUPPORTED_ERROR
PR_OPERATION_ABORTED_ERROR

Some of these may be returned by other socket I/O
functions than connect().  Some of these I don't
know what they mean exactly.

Sandeep: These if thrown by NSPR will now get converted into SSLSocketException.

Comment 16

[Wan-Teh Chang]

Comment on attachment 210632 [details] [diff] [review]
Modifications based on Alexei and Wan-Teh's feedback

r=wtc.  I have one suggested change.

In org/mozilla/jss/ssl/common.c

>+    switch (nativeErrcode) {
>+        case PR_PENDING_INTERRUPT_ERROR :
>+        case PR_OPERATION_ABORTED_ERROR :
>+            excepClass = (*env)->FindClass(env, INTERRUPTED_IO_EXCEPTION);
>+            break;

Remove PR_OPERATION_ABORTED_ERROR.

Comment 17

[Wan-Teh Chang]

Comment on attachment 210632 [details] [diff] [review]
Modifications based on Alexei and Wan-Teh's feedback

Some more review comments.

In org/mozilla/jss/ssl/SSLServerSocket.c

>+        } else if( err == PR_IO_ERROR ) {
>+            JSSL_throwSSLSocketException(env, 
>+                "Accept received IO error with error code " + err);
>         } else {
>-            JSSL_throwSSLSocketException
>-                (env, "Error accepting connection on server socket");
>+            JSSL_throwSSLSocketException(env, 
>+                "Connection accept failed with error code " + err);

It is better to replace "Accept" and "Connection accept" with
"Accept operation" to be consistent with the error messages for
the errors above.

In org/mozilla/jss/ssl/common.c

>+    excepCons = (*env)->GetMethodID(env, excepClass, "<init>",
>+        "(Ljava/lang/String;)V");
...
>+    excepObj = (*env)->NewObject(env, excepClass, excepCons, msgString);

I noticed that the arguments you passed to these two functions
are different from the previous patch.  I just want you to make
sure these two changes are not accidental.  In the previous patch,
you pass  "(Ljava/lang/String;I)V" as the fourth argument to
(*env)->GetMethodID, and you pass five arguments to (*env)->NewObject.

Comment 18

[Sandeep Konchady]

Wan-Teh,
  The changes to excepCons and excepObj were done based on a negative test that I wrote to test for exceptions in accept() and this change was needed.

Sandeep

Comment 19

[Sandeep Konchady]

Checking in JSS 4.1.x
Checking in org/mozilla/jss/ssl/SSLServerSocket.c;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/SSLServerSocket.c,v  <--  SSLServerSocket.c
new revision: 1.16.2.1; previous revision: 1.16
done
Checking in org/mozilla/jss/ssl/SSLServerSocket.java;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/SSLServerSocket.java,v  <--  SSLServerSocket.java
new revision: 1.20.2.1; previous revision: 1.20
done
Checking in org/mozilla/jss/ssl/common.c;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/common.c,v  <--  common.c
new revision: 1.23.2.2; previous revision: 1.23.2.1
done
Checking in org/mozilla/jss/util/jss_exceptions.h;
/cvsroot/mozilla/security/jss/org/mozilla/jss/util/jss_exceptions.h,v  <--  jss_exceptions.h
new revision: 1.11.2.1; previous revision: 1.11
done

Comment 20

[Sandeep Konchady]

Checking in code for JSS 4.2.x branch.

Checking in org/mozilla/jss/ssl/SSLServerSocket.c;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/SSLServerSocket.c,v  <--  SSLServerSocket.c
new revision: 1.17; previous revision: 1.16
done
Checking in org/mozilla/jss/ssl/SSLServerSocket.java;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/SSLServerSocket.java,v  <--  SSLServerSocket.java
new revision: 1.22; previous revision: 1.21
done
Checking in org/mozilla/jss/ssl/common.c;
/cvsroot/mozilla/security/jss/org/mozilla/jss/ssl/common.c,v  <--  common.c
new revision: 1.26; previous revision: 1.25
done
Checking in org/mozilla/jss/util/jss_exceptions.h;
/cvsroot/mozilla/security/jss/org/mozilla/jss/util/jss_exceptions.h,v  <--  jss_exceptions.h
new revision: 1.12; previous revision: 1.11
done