8000 Fix CompositeMetadataFlyweight check of ASCII only custom mime (#651) · FzNl/rsocket-java@c9a4ea1 · GitHub
[go: up one dir, main page]

Skip to content

Commit c9a4ea1

Browse files
simonbaslerobertroeser
authored andcommitted
Fix CompositeMetadataFlyweight check of ASCII only custom mime (rsocket#651)
* Fix CompositeMetadataFlyweight check of ASCII only custom mime This fixes potential false positives in the encodeMetadataHeader method "ASCII only" check that results in IllegalArgumentException. The first byte is readable but not actually written when performing the check, yet we would consider it in `isText` (looking at a random byte value). Signed-off-by: Simon Baslé <sbasle@pivotal.io> * add a test for the ascii check bug Signed-off-by: Simon Baslé <sbasle@pivotal.io> * forgot googleDataFormat (again) Signed-off-by: Simon Baslé <sbasle@pivotal.io>
1 parent d47b747 commit c9a4ea1

File tree

2 files changed

+39
-10
lines changed

2 files changed

+39
-10
lines changed

rsocket-core/src/main/java/io/rsocket/metadata/CompositeMetadataFlyweight.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,13 +329,15 @@ static ByteBuf encodeMetadataHeader(
329329
ByteBufAllocator allocator, String customMime, int metadataLength) {
330330
ByteBuf metadataHeader = allocator.buffer(4 + customMime.length());
331331
// reserve 1 byte for the customMime length
332+
// /!\ careful not to read that first byte, which is random at this point
332333
int writerIndexInitial = metadataHeader.writerIndex();
333334
metadataHeader.writerIndex(writerIndexInitial + 1);
334335

335336
// write the custom mime in UTF8 but validate it is all ASCII-compatible
336337
// (which produces the right result since ASCII chars are still encoded on 1 byte in UTF8)
337338
int customMimeLength = ByteBufUtil.writeUtf8(metadataHeader, customMime);
338-
if (!ByteBufUtil.isText(metadataHeader, CharsetUtil.US_ASCII)) {
339+
if (!ByteBufUtil.isText(
340+
metadataHeader, metadataHeader.readerIndex() + 1, customMimeLength, CharsetUtil.US_ASCII)) {
339341
metadataHeader.release();
340342
throw new IllegalArgumentException("custom mime type must be US_ASCII characters only");
341343
}

rsocket-core/src/test/java/io/rsocket/metadata/CompositeMetadataFlyweightTest.java

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,9 @@
1919
import static io.rsocket.metadata.CompositeMetadataFlyweight.decodeMimeAndContentBuffersSlices;
2020
import static io.rsocket.metadata.CompositeMetadataFlyweight.decodeMimeIdFromMimeBuffer;
2121
import static io.rsocket.metadata.CompositeMetadataFlyweight.decodeMimeTypeFromMimeBuffer;
22-
import static org.assertj.core.api.Assertions.assertThat;
23-
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
24-
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
25-
26-
import io.netty.buffer.ByteBuf;
27-
import io.netty.buffer.ByteBufAllocator;
28-
import io.netty.buffer.CompositeByteBuf;
29-
import io.netty.buffer.Unpooled;
30-
import io.netty.buffer.UnpooledByteBufAllocator;
22+
import static org.assertj.core.api.Assertions.*;
23+
24+
import io.netty.buffer.*;
3125
import io.netty.util.CharsetUtil;
3226
import io.rsocket.test.util.ByteBufUtils;
3327
import io.rsocket.util.NumberUtils;
@@ -524,4 +518,37 @@ void knownMimeHeaderZero_avro() {
524518

525519
assertThat(content.readableBytes()).as("no metadata content").isZero();
526520
}
521+
522+
@Test
523+
void encodeCustomHeaderAsciiCheckSkipsFirstByte() {
524+
final ByteBuf badBuf = Unpooled.copiedBuffer("é00000000000", CharsetUtil.UTF_8);
525+
badBuf.writerIndex(0);
526+
assertThat(badBuf.readerIndex()).isZero();
527+
528+
ByteBufAllocator allocator =
529+
new AbstractByteBufAllocator() {
530+
@Override
531+
public boolean isDirectBufferPooled() {
532+
return false;
533+
}
534+
535+
@Override
536+
protected ByteBuf newHeapBuffer(int initialCapacity, int maxCapacity) {
537+
return badBuf;
538+
}
539+
540+
@Override
541+
protected ByteBuf newDirectBuffer(int initialCapacity, int maxCapacity) {
542+
return badBuf;
543+
}
544+
};
545+
546+
assertThatCode(
547+
() -> CompositeMetadataFlyweight.encodeMetadataHeader(allocator, "custom/type", 0))
548+
.doesNotThrowAnyException();
549+
550+
assertThat(badBuf.readByte()).isEqualTo((byte) 10);
551+
assertThat(badBuf.readCharSequence(11, CharsetUtil.UTF_8)).hasToString("custom/type");
552+
assertThat(badBuf.readUnsignedMedium()).isEqualTo(0);
553+
}
527554
}

0 commit comments

Comments
 (0)
0