Skip to content

Commit 598e82f

Browse files
stevenschlanskerClaude (on behalf of Steven Schlansker)
andauthored
fix(format): pass row body size, not full payload size, to BinaryRow.pointTo (#3715)
## Why? Minor fixes - Latent bug in BinaryRowEncoder exposes 8 bytes beyond end of intended buffer to BinaryRow - Add reminder documentation for byte[] encoder forms why embedded size is ignored. Tripped up both me and Claude - Clean up docstring. Identified during AI review of forthcoming feature branches ## AI Contribution Checklist - [x] Substantial AI assistance was used in this PR: `yes` / `no` AI Usage Disclosure - substantial_ai_assistance: yes - scope: all - affected_files_or_subsystems: Java row format - ai_review: ✔️ - ai_review_artifacts: > ● Done. Three commits amended on fory-docs-encoders-cleanup: tightened comments, tightened test, tightened commit messages, no functional changes since the previous fix was already correct. Force-push when you're ready. - human_verification: ✔️ - performance_verification: N/A - provenance_license_confirmation: ✔️ ## Does this PR introduce any user-facing change? No --------- Co-authored-by: Claude (on behalf of Steven Schlansker) <stevenschlansker+claude@apache.org>
1 parent a5d1c77 commit 598e82f

5 files changed

Lines changed: 79 additions & 7 deletions

File tree

java/fory-format/src/main/java/org/apache/fory/format/encoder/BinaryArrayEncoder.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ public T decode(final MemoryBuffer buffer) {
6262

6363
@Override
6464
public T decode(final byte[] bytes) {
65+
// byte[] overloads ignore sizeEmbedded: encode writes no size prefix, decode uses bytes.length.
6566
return decode(MemoryUtils.wrap(bytes), bytes.length);
6667
}
6768

java/fory-format/src/main/java/org/apache/fory/format/encoder/BinaryMapEncoder.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ M decode(final MemoryBuffer buffer, final int size) {
8585

8686
@Override
8787
public M decode(final byte[] bytes) {
88+
// byte[] overloads ignore sizeEmbedded: encode writes no size prefix, decode uses bytes.length.
8889
return decode(MemoryUtils.wrap(bytes), bytes.length);
8990
}
9091

java/fory-format/src/main/java/org/apache/fory/format/encoder/BinaryRowEncoder.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,16 @@ T decode(final MemoryBuffer buffer, final int size) {
8181
+ "Please check writer schema.",
8282
schema, schemaHash, peerSchemaHash));
8383
}
84+
final int rowSize = size - 8;
8485
final BinaryRow row = codecFactory.newRow(schema);
85-
row.pointTo(buffer, buffer.readerIndex(), size);
86-
buffer.increaseReaderIndex(size - 8);
86+
row.pointTo(buffer, buffer.readerIndex(), rowSize);
87+
buffer.increaseReaderIndex(rowSize);
8788
return fromRow(row);
8889
}
8990

9091
@Override
9192
public T decode(final byte[] bytes) {
93+
// byte[] overloads ignore sizeEmbedded: encode writes no size prefix, decode uses bytes.length.
9294
return decode(MemoryUtils.wrap(bytes), bytes.length);
9395
}
9496

java/fory-format/src/main/java/org/apache/fory/format/encoder/Encoders.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,7 @@
4343
import org.apache.fory.type.TypeUtils;
4444
import org.apache.fory.util.Preconditions;
4545

46-
/**
47-
* Factory to create {@link Encoder}.
48-
*
49-
* <p>, ganrunsheng
50-
*/
46+
/** Factory to create {@link Encoder}. */
5147
public class Encoders {
5248
private static final Logger LOG = LoggerFactory.getLogger(Encoders.class);
5349

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.fory.format.encoder;
21+
22+
import lombok.Data;
23+
import org.apache.fory.format.row.binary.BinaryRow;
24+
import org.apache.fory.format.row.binary.writer.BinaryRowWriter;
25+
import org.apache.fory.format.type.Schema;
26+
import org.apache.fory.format.type.TypeInference;
27+
import org.testng.Assert;
28+
import org.testng.annotations.Test;
29+
30+
/**
31+
* BinaryRowEncoder.decode must record the row body size on BinaryRow, not the full payload size
32+
* including the leading 8-byte schema hash.
33+
*/
34+
public class BinaryRowEncoderPointToTest {
35+
36+
@Data
37+
public static class Tiny {
38+
private int x;
39+
}
40+
41+
@Test
42+
public void decodedRowSizeMatchesRowBody() {
43+
Schema schema = TypeInference.inferSchema(Tiny.class);
44+
RowEncoder<Tiny> real = Encoders.bean(Tiny.class);
45+
Tiny in = new Tiny();
46+
in.setX(42);
47+
byte[] payload = real.encode(in);
48+
49+
BinaryRow[] captured = new BinaryRow[1];
50+
GeneratedRowEncoder captor =
51+
new GeneratedRowEncoder() {
52+
@Override
53+
public BinaryRow toRow(Object obj) {
54+
return real.toRow((Tiny) obj);
55+
}
56+
57+
@Override
58+
public Object fromRow(BinaryRow row) {
59+
captured[0] = row;
60+
return null;
61+
}
62+
};
63+
64+
BinaryRowEncoder<Tiny> encoder =
65+
new BinaryRowEncoder<>(
66+
schema, DefaultCodecFormat.INSTANCE, captor, new BinaryRowWriter(schema), false);
67+
encoder.decode(payload);
68+
69+
Assert.assertNotNull(captured[0], "decode must hand the row to fromRow");
70+
Assert.assertEquals(captured[0].getSizeInBytes(), payload.length - 8);
71+
}
72+
}

0 commit comments

Comments
 (0)