Skip to content

Commit ea2b570

Browse files
DavidiusdadiDavidiusdadi
committed
SSLSocketChannel2 refactoring: made read and write operations not work
on the same SSLEngineResult and use sslEngine directly to obtain HandshakeStatus
1 parent 5d7a1fe commit ea2b570

File tree

1 file changed

+33
-22
lines changed

1 file changed

+33
-22
lines changed

‎src/main/java/org/java_websocket/SSLSocketChannel2.java‎

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
* Implements the relevant portions of the SocketChannel interface with the SSLEngine wrapper.
3232
*/
3333
public class SSLSocketChannel2 implements ByteChannel, WrappedByteChannel {
34+
/**
35+
* This object is used to feed the {@link SSLEngine}'s wrap and unwrap methods during the handshake phase.
36+
**/
3437
protected static ByteBuffer emptybuffer = ByteBuffer.allocate( 0 );
3538

3639
protected ExecutorService exec;
@@ -49,12 +52,9 @@ public class SSLSocketChannel2 implements ByteChannel, WrappedByteChannel {
4952
/** used to set interestOP SelectionKey.OP_WRITE for the underlying channel */
5053
protected SelectionKey selectionKey;
5154

52-
53-
protected SSLEngineResult engineResult;
5455
protected SSLEngine sslEngine;
55-
56-
57-
private Status engineStatus = Status.BUFFER_UNDERFLOW;
56+
protected SSLEngineResult readEngineResult;
57+
protected SSLEngineResult writeEngineResult;
5858

5959
public SSLSocketChannel2( SocketChannel channel , SSLEngine sslEngine , ExecutorService exec , SelectionKey key ) throws IOException {
6060
if( channel == null || sslEngine == null || exec == null )
@@ -64,6 +64,8 @@ public SSLSocketChannel2( SocketChannel channel , SSLEngine sslEngine , Executor
6464
this.sslEngine = sslEngine;
6565
this.exec = exec;
6666

67+
readEngineResult = writeEngineResult = new SSLEngineResult( Status.BUFFER_UNDERFLOW, sslEngine.getHandshakeStatus(), 0, 0 ); // init to prevent NPEs
68+
6769
tasks = new ArrayList<Future<?>>( 3 );
6870
if( key != null ) {
6971
key.interestOps( key.interestOps() | SelectionKey.OP_WRITE );
@@ -93,8 +95,12 @@ private void consumeFutureUninterruptible( Future<?> f ) {
9395
}
9496
}
9597

98+
/**
99+
* This method will do whatever necessary to process the sslengine handshake.
100+
* Thats why it's called both from the {@link #read(ByteBuffer)} and {@link #write(ByteBuffer)}
101+
**/
96102
private synchronized void processHandshake() throws IOException {
97-
if( engineResult.getHandshakeStatus() == HandshakeStatus.NOT_HANDSHAKING )
103+
if( sslEngine.getHandshakeStatus() == HandshakeStatus.NOT_HANDSHAKING )
98104
return; // since this may be called either from a reading or a writing thread and because this method is synchronized it is necessary to double check if we are still handshaking.
99105
if( !tasks.isEmpty() ) {
100106
Iterator<Future<?>> it = tasks.iterator();
@@ -110,8 +116,8 @@ private synchronized void processHandshake() throws IOException {
110116
}
111117
}
112118

113-
if( engineResult.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_UNWRAP ) {
114-
if( !isBlocking() || engineStatus == Status.BUFFER_UNDERFLOW ) {
119+
if( sslEngine.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_UNWRAP ) {
120+
if( !isBlocking() || readEngineResult.getStatus() == Status.BUFFER_UNDERFLOW ) {
115121
inCrypt.compact();
116122
int read = socketChannel.read( inCrypt );
117123
if( read == -1 ) {
@@ -121,35 +127,37 @@ private synchronized void processHandshake() throws IOException {
121127
}
122128
inData.compact();
123129
unwrap();
124-
if( engineResult.getHandshakeStatus() == HandshakeStatus.FINISHED ) {
130+
if( sslEngine.getHandshakeStatus() == HandshakeStatus.FINISHED ) {
125131
createBuffers( sslEngine.getSession() );
126132
return;
127133
}
128134
}
129135
consumeDelegatedTasks();
130-
assert ( engineResult.getHandshakeStatus() != HandshakeStatus.NOT_HANDSHAKING );
131-
if( tasks.isEmpty() || engineResult.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_WRAP ) {
136+
assert ( sslEngine.getHandshakeStatus() != HandshakeStatus.NOT_HANDSHAKING );
137+
if( tasks.isEmpty() || sslEngine.getHandshakeStatus() == SSLEngineResult.HandshakeStatus.NEED_WRAP ) {
132138
socketChannel.write( wrap( emptybuffer ) );
133-
if( engineResult.getHandshakeStatus() == HandshakeStatus.FINISHED ) {
139+
if( sslEngine.getHandshakeStatus() == HandshakeStatus.FINISHED ) {
134140
createBuffers( sslEngine.getSession() );
135141
}
136142
}
137143
}
138144

139145
private synchronized ByteBuffer wrap( ByteBuffer b ) throws SSLException {
140146
outCrypt.compact();
141-
engineResult = sslEngine.wrap( b, outCrypt );
147+
writeEngineResult = sslEngine.wrap( b, outCrypt );
142148
outCrypt.flip();
143149
return outCrypt;
144150
}
145151

152+
/**
153+
* performs the unwrap operation by unwrapping from {@link #inCrypt} to {@link #inData}
154+
**/
146155
private synchronized ByteBuffer unwrap() throws SSLException {
147156
int rem;
148157
do {
149158
rem = inData.remaining();
150-
engineResult = sslEngine.unwrap( inCrypt, inData );
151-
engineStatus = engineResult.getStatus();
152-
} while ( engineStatus == SSLEngineResult.Status.OK && ( rem != inData.remaining() || engineResult.getHandshakeStatus() == HandshakeStatus.NEED_UNWRAP ) );
159+
readEngineResult = sslEngine.unwrap( inCrypt, inData );
160+
} while ( readEngineResult.getStatus() == SSLEngineResult.Status.OK && ( rem != inData.remaining() || sslEngine.getHandshakeStatus() == HandshakeStatus.NEED_UNWRAP ) );
153161

154162
inData.flip();
155163
return inData;
@@ -200,6 +208,8 @@ public int write( ByteBuffer src ) throws IOException {
200208
/**
201209
* Blocks when in blocking mode until at least one byte has been decoded.<br>
202210
* When not in blocking mode 0 may be returned.
211+
*
212+
* @return the number of bytes read.
203213
**/
204214
public int read( ByteBuffer dst ) throws IOException {
205215
if( !dst.hasRemaining() )
@@ -216,7 +226,6 @@ public int read( ByteBuffer dst ) throws IOException {
216226
}
217227
}
218228
}
219-
220229
int purged = readRemaining( dst );
221230
if( purged != 0 )
222231
return purged;
@@ -229,7 +238,7 @@ public int read( ByteBuffer dst ) throws IOException {
229238
else
230239
inCrypt.compact();
231240

232-
if( ( isBlocking() && inCrypt.position() == 0 ) || engineStatus == Status.BUFFER_UNDERFLOW )
241+
if( ( isBlocking() && inCrypt.position() == 0 ) || readEngineResult.getStatus() == Status.BUFFER_UNDERFLOW )
233242
if( socketChannel.read( inCrypt ) == -1 ) {
234243
return -1;
235244
}
@@ -243,6 +252,9 @@ public int read( ByteBuffer dst ) throws IOException {
243252
return transfered;
244253
}
245254

255+
/**
256+
* {@link #read(ByteBuffer)} may not be to leave all buffers(inData, inCrypt)
257+
**/
246258
private int readRemaining( ByteBuffer dst ) throws SSLException {
247259
if( inData.hasRemaining() ) {
248260
return transfereTo( inData, dst );
@@ -273,7 +285,7 @@ public void close() throws IOException {
273285
}
274286

275287
private boolean isHandShakeComplete() {
276-
HandshakeStatus status = engineResult.getHandshakeStatus();
288+
HandshakeStatus status = sslEngine.getHandshakeStatus();
277289
return status == SSLEngineResult.HandshakeStatus.FINISHED || status == SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING;
278290
}
279291

@@ -304,7 +316,7 @@ public boolean isOpen() {
304316

305317
@Override
306318
public boolean isNeedWrite() {
307-
return outCrypt.hasRemaining() || !isHandShakeComplete();
319+
return outCrypt.hasRemaining() || !isHandShakeComplete(); // FIXME this condition can cause high cpu load during handshaking when network is slow
308320
}
309321

310322
@Override
@@ -314,8 +326,7 @@ public void writeMore() throws IOException {
314326

315327
@Override
316328
public boolean isNeedRead() {
317-
return inData.hasRemaining() || ( inCrypt.hasRemaining() && engineResult.getStatus() != Status.BUFFER_UNDERFLOW &&
318-
engineResult.getStatus() != Status.CLOSED );
329+
return inData.hasRemaining() || ( inCrypt.hasRemaining() && readEngineResult.getStatus() != Status.BUFFER_UNDERFLOW && readEngineResult.getStatus() != Status.CLOSED );
319330
}
320331

321332
@Override

0 commit comments

Comments
 (0)