Skip to content

Commit c7408cb

Browse files
kareskares
committed
[test] replace internal Ruby test with Java unit test
- writeNonblockDataIntegrity: approximates the gem push scenario from #242 — large payload via write_nonblock loop, then read server's byte count response, assert data integrity (no bytes lost) - writeNonblockNetWriteDataState: saturates TCP buffer, then accesses the package-private netWriteData field directly to verify buffer consistency after the compact() fix
1 parent 1b6be67 commit c7408cb

File tree

6 files changed

+257
-98
lines changed

6 files changed

+257
-98
lines changed

‎Mavenfile‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,12 @@ plugin :clean do
8282
'failOnError' => 'false' )
8383
end
8484

85-
jar 'org.jruby:jruby-core', '9.2.0.0', :scope => :provided
85+
jruby_compile_compat = '9.2.0.0'
86+
jar 'org.jruby:jruby-core', jruby_compile_compat, :scope => :provided
8687
# for invoker generated classes we need to add javax.annotation when on Java > 8
8788
jar 'javax.annotation:javax.annotation-api', '1.3.1', :scope => :compile
89+
# a test dependency to provide digest and other stdlib bits, needed when loading OpenSSL in Java unit tests
90+
jar 'org.jruby:jruby-stdlib', jruby_compile_compat, :scope => :test
8891
jar 'junit:junit', '[4.13.1,)', :scope => :test
8992

9093
# NOTE: to build on Java 11 - installing gems fails (due old jossl) with:

‎pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,12 @@ DO NOT MODIFY - GENERATED CODE
107107
<version>1.3.1</version>
108108
<scope>compile</scope>
109109
</dependency>
110+
<dependency>
111+
<groupId>org.jruby</groupId>
112+
<artifactId>jruby-stdlib</artifactId>
113+
<version>9.2.0.0</version>
114+
<scope>test</scope>
115+
</dependency>
110116
<dependency>
111117
<groupId>junit</groupId>
112118
<artifactId>junit</artifactId>

‎src/main/java/org/jruby/ext/openssl/SSLSocket.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,9 @@ private static CallSite callSite(final CallSite[] sites, final CallSiteIndex ind
147147
private SSLEngine engine;
148148
private RubyIO io;
149149

150-
private ByteBuffer appReadData;
151-
private ByteBuffer netReadData;
152-
private ByteBuffer netWriteData;
150+
ByteBuffer appReadData;
151+
ByteBuffer netReadData;
152+
ByteBuffer netWriteData;
153153

154154
private boolean initialHandshake = false;
155155
private transient long initializeTime;

‎src/test/java/org/jruby/ext/openssl/SSLSocketTest.java‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
package org.jruby.ext.openssl;
2+
3+
import java.io.IOException;
4+
import java.io.ByteArrayOutputStream;
5+
import java.io.InputStream;
6+
import java.nio.ByteBuffer;
7+
import java.nio.charset.StandardCharsets;
8+
9+
import org.jruby.Ruby;
10+
import org.jruby.RubyArray;
11+
import org.jruby.RubyFixnum;
12+
import org.jruby.RubyInteger;
13+
import org.jruby.RubyString;
14+
import org.jruby.exceptions.RaiseException;
15+
import org.jruby.runtime.ThreadContext;
16+
import org.jruby.runtime.builtin.IRubyObject;
17+
18+
import org.junit.After;
19+
import org.junit.Before;
20+
import org.junit.Test;
21+
import static org.junit.Assert.*;
22+
23+
public class SSLSocketTest {
24+
25+
private Ruby runtime;
26+
27+
/** Loads the ssl_pair.rb script that creates a connected SSL socket pair. */
28+
private String start_ssl_server_rb() { return readResource("/start_ssl_server.rb"); }
29+
30+
@Before
31+
public void setUp() {
32+
runtime = Ruby.newInstance();
33+
// prepend lib/ so openssl.rb + jopenssl/ are loaded instead of the ones bundled in jruby-stdlib
34+
String libDir = new java.io.File("lib").getAbsolutePath();
35+
runtime.evalScriptlet("$LOAD_PATH.unshift '" + libDir + "'");
36+
runtime.evalScriptlet("require 'openssl'");
37+
}
38+
39+
@After
40+
public void tearDown() {
41+
if (runtime != null) {
42+
runtime.tearDown(false);
43+
runtime = null;
44+
}
45+
}
46+
47+
/**
48+
* Real-world scenario: {@code gem push} sends a large POST body via {@code syswrite_nonblock},
49+
* then reads the HTTP response via {@code sysread}.
50+
*
51+
* Approximates the {@code gem push} scenario:
52+
* <ol>
53+
* <li>Write 256KB via {@code syswrite_nonblock} in a loop (the net/http POST pattern)</li>
54+
* <li>Server reads via {@code sysread} and counts bytes</li>
55+
* <li>Assert: server received exactly what client sent</li>
56+
* </ol>
57+
*
58+
* With the old {@code clear()} bug, encrypted bytes were silently
59+
* discarded during partial non-blocking writes, so the server would
60+
* receive fewer bytes than sent.
61+
*/
62+
@Test
63+
public void syswriteNonblockDataIntegrity() throws Exception {
64+
final RubyArray pair = (RubyArray) runtime.evalScriptlet(start_ssl_server_rb());
65+
SSLSocket client = (SSLSocket) pair.entry(0).toJava(SSLSocket.class);
66+
SSLSocket server = (SSLSocket) pair.entry(1).toJava(SSLSocket.class);
67+
68+
try {
69+
// Server: read all data in a background thread, counting bytes
70+
final long[] serverReceived = { 0 };
71+
Thread serverReader = startServerReader(server, serverReceived);
72+
73+
// Client: write 256KB in 4KB chunks via syswrite_nonblock
74+
byte[] chunk = new byte[4096];
75+
java.util.Arrays.fill(chunk, (byte) 'P'); // P for POST body
76+
RubyString payload = RubyString.newString(runtime, chunk);
77+
78+
long totalSent = 0;
79+
for (int i = 0; i < 64; i++) { // 64 * 4KB = 256KB
80+
try {
81+
IRubyObject written = client.syswrite_nonblock(currentContext(), payload);
82+
totalSent += ((RubyInteger) written).getLongValue();
83+
} catch (RaiseException e) {
84+
String rubyClass = e.getException().getMetaClass().getName();
85+
if (rubyClass.contains("WaitWritable")) {
86+
// Expected: non-blocking write would block — retry as blocking
87+
IRubyObject written = client.syswrite(currentContext(), payload);
88+
totalSent += ((RubyInteger) written).getLongValue();
89+
} else {
90+
System.err.println("syswrite_nonblock unexpected: " + rubyClass + ": " + e.getMessage());
91+
throw e;
92+
}
93+
}
94+
}
95+
assertTrue("should have sent data", totalSent > 0);
96+
97+
// Close client to signal EOF, let server finish reading
98+
client.callMethod(currentContext(), "close");
99+
serverReader.join(10_000);
100+
101+
assertEquals(
102+
"server must receive exactly what client sent — mismatch means encrypted bytes were lost!",
103+
totalSent, serverReceived[0]
104+
);
105+
} finally {
106+
closeQuietly(pair);
107+
}
108+
}
109+
110+
private Thread startServerReader(final SSLSocket server, final long[] serverReceived) {
111+
Thread serverReader = new Thread(() -> {
112+
try {
113+
RubyFixnum len = RubyFixnum.newFixnum(runtime, 8192);
114+
while (true) {
115+
IRubyObject data = server.sysread(currentContext(), len);
116+
serverReceived[0] += ((RubyString) data).getByteList().getRealSize();
117+
}
118+
} catch (RaiseException e) {
119+
String rubyClass = e.getException().getMetaClass().getName();
120+
// EOFError or IOError expected when client closes the connection
121+
if (!rubyClass.equals("EOFError") && !rubyClass.equals("IOError")) {
122+
System.err.println("server reader unexpected: " + rubyClass + ": " + e.getMessage());
123+
e.printStackTrace(System.err);
124+
}
125+
}
126+
});
127+
serverReader.start();
128+
return serverReader;
129+
}
130+
131+
/**
132+
* After saturating the TCP send buffer with {@code syswrite_nonblock},
133+
* inspect {@code netWriteData} to verify the buffer is consistent.
134+
*/
135+
@Test
136+
public void syswriteNonblockNetWriteDataConsistency() {
137+
final RubyArray pair = (RubyArray) runtime.evalScriptlet(start_ssl_server_rb());
138+
SSLSocket client = (SSLSocket) pair.entry(0).toJava(SSLSocket.class);
139+
140+
try {
141+
assertNotNull("netWriteData initialized after handshake", client.netWriteData);
142+
143+
// Saturate: server is not reading yet, so backpressure builds
144+
byte[] chunk = new byte[16384];
145+
java.util.Arrays.fill(chunk, (byte) 'S');
146+
RubyString payload = RubyString.newString(runtime, chunk);
147+
148+
int successfulWrites = 0;
149+
for (int i = 0; i < 200; i++) {
150+
try {
151+
client.syswrite_nonblock(currentContext(), payload);
152+
successfulWrites++;
153+
} catch (RaiseException e) {
154+
String rubyClass = e.getException().getMetaClass().getName();
155+
if (rubyClass.contains("WaitWritable") || rubyClass.equals("IOError")) {
156+
break; // buffer saturated — expected
157+
}
158+
System.err.println("saturate loop unexpected: " + rubyClass + ": " + e.getMessage());
159+
throw e;
160+
}
161+
}
162+
assertTrue("at least one write should succeed", successfulWrites > 0);
163+
164+
// Inspect netWriteData directly
165+
ByteBuffer netWriteData = client.netWriteData;
166+
assertTrue("position <= limit", netWriteData.position() <= netWriteData.limit());
167+
assertTrue("limit <= capacity", netWriteData.limit() <= netWriteData.capacity());
168+
169+
// If there are unflushed bytes, compact() preserved them
170+
if (netWriteData.remaining() > 0) {
171+
// The bytes should be valid TLS record data, not zeroed memory
172+
byte b = netWriteData.get(netWriteData.position());
173+
assertNotEquals("preserved bytes should be TLS data, not zeroed", 0, b);
174+
}
175+
176+
} finally {
177+
closeQuietly(pair);
178+
}
179+
}
180+
181+
private ThreadContext currentContext() {
182+
return runtime.getCurrentContext();
183+
}
184+
185+
private void closeQuietly(final RubyArray sslPair) {
186+
for (int i = 0; i < sslPair.getLength(); i++) {
187+
try { sslPair.entry(i).callMethod(currentContext(), "close"); }
188+
catch (RaiseException e) { /* already closed */ }
189+
}
190+
}
191+
192+
static String readResource(final String resource) {
193+
int n;
194+
try (InputStream in = SSLSocketTest.class.getResourceAsStream(resource)) {
195+
if (in == null) throw new IllegalArgumentException(resource + " not found on classpath");
196+
197+
ByteArrayOutputStream out = new ByteArrayOutputStream();
198+
byte[] buf = new byte[8192];
199+
while ((n = in.read(buf)) != -1) out.write(buf, 0, n);
200+
return new String(out.toByteArray(), StandardCharsets.UTF_8);
201+
} catch (IOException e) {
202+
throw new IllegalStateException("failed to load" + resource, e);
203+
}
204+
}
205+
}

‎src/test/resources/start_ssl_server.rb‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Creates a connected SSL socket pair for Java unit tests.
2+
# Returns [client_ssl, server_ssl]
3+
#
4+
# OpenSSL extension is loaded by SSLSocketTest.setUp via OpenSSL.load(runtime).
5+
6+
require 'socket'
7+
8+
key = OpenSSL::PKey::RSA.new(2048)
9+
cert = OpenSSL::X509::Certificate.new
10+
cert.version = 2
11+
cert.serial = 1
12+
cert.subject = cert.issuer = OpenSSL::X509::Name.parse('/CN=Test')
13+
cert.public_key = key.public_key
14+
cert.not_before = Time.now
15+
cert.not_after = Time.now + 3600
16+
cert.sign(key, OpenSSL::Digest::SHA256.new)
17+
18+
tcp_server = TCPServer.new('127.0.0.1', 0)
19+
port = tcp_server.local_address.ip_port
20+
ctx = OpenSSL::SSL::SSLContext.new
21+
ctx.cert = cert
22+
ctx.key = key
23+
ssl_server = OpenSSL::SSL::SSLServer.new(tcp_server, ctx)
24+
ssl_server.start_immediately = true
25+
26+
server_ssl = nil
27+
server_thread = Thread.new { server_ssl = ssl_server.accept }
28+
29+
sock = TCPSocket.new('127.0.0.1', port)
30+
sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 4096)
31+
sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_RCVBUF, 4096)
32+
client_ssl = OpenSSL::SSL::SSLSocket.new(sock)
33+
client_ssl.sync_close = true
34+
client_ssl.connect
35+
server_thread.join(5)
36+
37+
[client_ssl, server_ssl]

‎src/test/ruby/ssl/test_write_nonblock.rb‎

Lines changed: 2 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -192,98 +192,6 @@ def test_many_small_write_nonblock_calls
192192
end
193193
end
194194

195-
# Detect the netWriteData.clear() bug by invoking the Java write() directly, bypassing syswriteImpl's `waitSelect`.
196-
#
197-
# Reproducer for #242 (jruby/jruby#8935).
198-
#
199-
# Strategy:
200-
# 1. Saturate the TCP send buffer (server doesn't read)
201-
# 2. Call write(ByteBuffer, false) directly via Java reflection
202-
# 3. Check netWriteData.remaining() — if > 0, data would be discarded by the next write() call's netWriteData.clear()
203-
def test_internal_write_nonblock_unflushed_data_detected
204-
require 'socket'
205-
206-
tcp_server = TCPServer.new("127.0.0.1", 0)
207-
port = tcp_server.local_address.ip_port
208-
209-
server_ctx = OpenSSL::SSL::SSLContext.new
210-
server_ctx.cert = @svr_cert
211-
server_ctx.key = @svr_key
212-
server_ctx.min_version = server_ctx.max_version = OpenSSL::SSL::TLS1_3_VERSION
213-
214-
ssl_server = OpenSSL::SSL::SSLServer.new(tcp_server, server_ctx)
215-
ssl_server.start_immediately = true
216-
217-
server_thread = Thread.new do
218-
Thread.current.report_on_exception = false
219-
begin
220-
ssl_conn = ssl_server.accept
221-
sleep 30 # Do NOT read — maximize backpressure
222-
ssl_conn.close rescue nil
223-
rescue
224-
end
225-
end
226-
227-
begin
228-
sock = TCPSocket.new("127.0.0.1", port)
229-
sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_SNDBUF, 4096)
230-
sock.setsockopt(Socket::SOL_SOCKET, Socket::SO_RCVBUF, 4096)
231-
232-
ssl = OpenSSL::SSL::SSLSocket.new(sock)
233-
ssl.sync_close = true
234-
ssl.connect
235-
236-
java_cls = Java::OrgJrubyExtOpenssl::SSLSocket.java_class
237-
java_ssl = ssl.to_java(Java::OrgJrubyExtOpenssl::SSLSocket)
238-
239-
nwd_field = java_cls.declared_field("netWriteData")
240-
nwd_field.accessible = true
241-
# Get the write(ByteBuffer, boolean) method via reflection
242-
write_method = java_cls.declared_method("write", java.nio.ByteBuffer.java_class, Java::boolean)
243-
write_method.accessible = true
244-
245-
# Phase 1: fill the TCP send buffer via normal write_nonblock
246-
chunk = "H" * 16384
247-
100.times do
248-
begin
249-
ssl.write_nonblock(chunk)
250-
rescue IO::WaitWritable, OpenSSL::SSL::SSLErrorWaitWritable
251-
break
252-
rescue IOError, OpenSSL::SSL::SSLError
253-
break
254-
end
255-
end
256-
257-
# Phase 2: call write(src, false) directly — this bypasses
258-
# syswriteImpl's waitSelect and goes straight to the code path
259-
# that has the clear() bug.
260-
src = java.nio.ByteBuffer.wrap(("I" * 4096).to_java_bytes)
261-
begin
262-
write_method.invoke(java_ssl, src, false)
263-
rescue Java::JavaLangReflect::InvocationTargetException => e
264-
warn "write() threw: #{e.cause}" if $VERBOSE # Expected — write may throw due to the saturated buffer
265-
end
266-
267-
nwd = nwd_field.value(java_ssl)
268-
remaining = nwd.remaining
269-
270-
if remaining > 0
271-
# BUG CONFIRMED: there are unflushed encrypted bytes in netWriteData.
272-
# The next write() call will do netWriteData.clear(), discarding them.
273-
# This is exactly the data loss bug from issue #242.
274-
#
275-
# To prove actual data loss, we would call write() again — the clear() would discard remaining encrypted bytes,
276-
# corrupting the TLS record stream and eventually causing the server to see fewer bytes than the client sent.
277-
assert remaining > 0, "netWriteData has #{remaining} unflushed bytes — next write() would discard them via clear()"
278-
else
279-
omit "Could not produce unflushed netWriteData in loopback (remaining=#{remaining}); bug requires network latency"
280-
end
281-
ensure
282-
ssl.close rescue nil
283-
sock.close rescue nil
284-
tcp_server.close rescue nil
285-
server_thread.kill rescue nil
286-
server_thread.join(2) rescue nil
287-
end
288-
end if defined?(JRUBY_VERSION)
195+
# NOTE: the netWriteData compact-vs-clear unit test for #242 (jruby/jruby#8935) is now a
196+
# Java test in SSLSocketTest — it can access package-private state directly without reflection.
289197
end

0 commit comments

Comments
 (0)