Skip to content

Commit fa4e28b

Browse files
kachayevnormanmaurer
authored andcommitted
Fix random number generators in WebSocketUtil
Motivation: Implementation of WebSocketUtil/randomNumber is incorrect and might violate the API returning values > maximum specified. Modifications: * WebSocketUtil/randomNumber is reimplemented, the idea of the solution described in the comment in the code * Implementation of WebSocketUtil/randomBytes changed to nextBytes method * PlatformDependet.threadLocalRandom is used instead of Math.random to improve efficiency * Added test cases to check random numbers generator * To ensure corretness, we now assert that min < max when generating random number Result: WebSocketUtil/randomNumber always produces correct result. Covers netty#8023
1 parent 9ffdec3 commit fa4e28b

File tree

3 files changed

+70
-8
lines changed

3 files changed

+70
-8
lines changed

codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketUtil.java

+25-6
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import io.netty.handler.codec.base64.Base64;
2121
import io.netty.util.CharsetUtil;
2222
import io.netty.util.concurrent.FastThreadLocal;
23+
import io.netty.util.internal.PlatformDependent;
2324

2425
import java.security.MessageDigest;
2526
import java.security.NoSuchAlgorithmException;
@@ -105,11 +106,7 @@ static String base64(byte[] data) {
105106
*/
106107
static byte[] randomBytes(int size) {
107108
byte[] bytes = new byte[size];
108-
109-
for (int index = 0; index < size; index++) {
110-
bytes[index] = (byte) randomNumber(0, 255);
111-
}
112-
109+
PlatformDependent.threadLocalRandom().nextBytes(bytes);
113110
return bytes;
114111
}
115112

@@ -121,7 +118,29 @@ static byte[] randomBytes(int size) {
121118
* @return A pseudo-random number
122119
*/
123120
static int randomNumber(int minimum, int maximum) {
124-
return (int) (Math.random() * maximum + minimum);
121+
assert minimum < maximum;
122+
double fraction = PlatformDependent.threadLocalRandom().nextDouble();
123+
124+
// the idea here is that nextDouble gives us a random value
125+
//
126+
// 0 <= fraction <= 1
127+
//
128+
// the distance from min to max declared as
129+
//
130+
// dist = max - min
131+
//
132+
// satisfies the following
133+
//
134+
// min + dist = max
135+
//
136+
// taking into account
137+
//
138+
// 0 <= fraction * dist <= dist
139+
//
140+
// we've got
141+
//
142+
// min <= min + fraction * dist <= max
143+
return (int) (minimum + fraction * (maximum - minimum));
125144
}
126145

127146
/**

codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,8 @@ protected WebSocketFrameEncoder newWebSocketEncoder() {
240240
}
241241
};
242242

243-
byte[] data = new byte[24];
244-
PlatformDependent.threadLocalRandom().nextBytes(data);
243+
// use randomBytes helper from utils to check that it functions properly
244+
byte[] data = WebSocketUtil.randomBytes(24);
245245

246246
// Create a EmbeddedChannel which we will use to encode a BinaryWebsocketFrame to bytes and so use these
247247
// to test the actual handshaker.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/*
2+
* Copyright 2018 The Netty Project
3+
*
4+
* The Netty Project licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
package io.netty.handler.codec.http.websocketx;
17+
18+
import org.junit.Test;
19+
20+
import static org.junit.Assert.assertTrue;
21+
22+
public class WebSocketUtilTest {
23+
24+
// how many times do we want to run each random variable checker
25+
private static final int NUM_ITERATIONS = 1000;
26+
27+
private static void assertRandomWithinBoundaries(int min, int max) {
28+
int r = WebSocketUtil.randomNumber(min, max);
29+
assertTrue(min <= r && r <= max);
30+
}
31+
32+
@Test
33+
public void testRandomNumberGenerator() {
34+
int iteration = 0;
35+
while (++iteration < NUM_ITERATIONS) {
36+
assertRandomWithinBoundaries(0, 1);
37+
assertRandomWithinBoundaries(0, 1);
38+
assertRandomWithinBoundaries(-1, 1);
39+
assertRandomWithinBoundaries(-1, 0);
40+
}
41+
}
42+
43+
}

0 commit comments

Comments
 (0)