Skip to content

Commit

Permalink
Avoid write target modifying shared src bytes
Browse files Browse the repository at this point in the history
When the incoming bytes are on the heap, we can use them in-place
for the string we pass to the Ruby write method. However, since
the target IO-like may want to modify the incoming string, we must
make sure it is marked as shared so our original src bytes will
not be modified.

This issue originally manifested in jruby#4903 and was fixed by always
copying the incoming buffer, but marking the string shared has the
same effecit if the target attempts to make any modifications.
  • Loading branch information
headius committed Feb 12, 2021
1 parent af07296 commit fdb251b
Showing 1 changed file with 19 additions and 15 deletions.
34 changes: 19 additions & 15 deletions core/src/main/java/org/jruby/util/IOChannel.java
Expand Up @@ -50,11 +50,11 @@
* @see IOReadableWritableByteChannel
*/
public abstract class IOChannel implements Channel {
private final IRubyObject io;
protected final IRubyObject io;
private final CallSite closeAdapter = MethodIndex.getFunctionalCallSite("close");
private final RespondToCallSite respondToClosed = new RespondToCallSite("closed?");
private final CallSite isClosedAdapter = MethodIndex.getFunctionalCallSite("closed?");
private final Ruby runtime;
protected final Ruby runtime;

protected IOChannel(final IRubyObject io) {
this.io = io;
Expand Down Expand Up @@ -83,7 +83,7 @@ public boolean isOpen() {
return true;
}

protected int read(CallSite read, ByteBuffer dst) throws IOException {
protected static int read(Ruby runtime, IRubyObject io, CallSite read, ByteBuffer dst) {
IRubyObject readValue = read.call(runtime.getCurrentContext(), io, io, runtime.newFixnum(dst.remaining()));
int returnValue = -1;
if (!readValue.isNil()) {
Expand All @@ -94,23 +94,27 @@ protected int read(CallSite read, ByteBuffer dst) throws IOException {
return returnValue;
}

protected int write(CallSite write, ByteBuffer src) throws IOException {
ByteList buffer;
protected static int write(Ruby runtime, IRubyObject io, CallSite write, ByteBuffer src) {
RubyString string;
int position = src.position();
int remaining = src.remaining();

// wrap buffer contents with ByteList
// wrap buffer contents with String
if (src.hasArray()) {
buffer = new ByteList(src.array(), src.position(), src.remaining(), false);
// wrap heap src with shared string to prevent target from modifying our buffer (GH-4903)
string = RubyString.newStringShared(runtime, src.array(), position, remaining);
} else {
buffer = new ByteList(src.remaining());
buffer.append(src, src.remaining());
// copy native src to heap bytes to use in string
byte[] bytes = new byte[remaining];
src.duplicate().get(bytes);
string = RubyString.newStringNoCopy(runtime, bytes);
}

// call write with new String based on this ByteList
IRubyObject written = write.call(runtime.getCurrentContext(), io, io, RubyString.newStringLight(runtime, buffer));
IRubyObject written = write.call(runtime.getCurrentContext(), io, io, string);
int wrote = written.convertToInteger().getIntValue();

// advance source buffer to match bytes written
// set source position to match bytes written
if (wrote > 0) {
src.position(position + wrote);
}
Expand Down Expand Up @@ -154,7 +158,7 @@ public IOReadableByteChannel(final IRubyObject io, final String readMethod) {
}

public int read(ByteBuffer dst) throws IOException {
return read(read, dst);
return read(runtime, io, read, dst);
}
}

Expand All @@ -170,7 +174,7 @@ public IOWritableByteChannel(final IRubyObject io) {
}

public int write(ByteBuffer src) throws IOException {
return write(write, src);
return write(runtime, io, write, src);
}
}

Expand All @@ -188,11 +192,11 @@ public IOReadableWritableByteChannel(final IRubyObject io) {
}

public int read(ByteBuffer dst) throws IOException {
return read(read, dst);
return read(runtime, io, read, dst);
}

public int write(ByteBuffer src) throws IOException {
return write(write, src);
return write(runtime, io, write, src);
}
}
}

0 comments on commit fdb251b

Please sign in to comment.