This is an automated email from the ASF dual-hosted git repository.
terrymanu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shardingsphere.git
The following commit(s) were added to refs/heads/master by this push:
new 0ea09c91754 Fix firebird accept data padding (#40188)
0ea09c91754 is described below
commit 0ea09c91754a7358eb8748dcdc69194e49083e74
Author: Maxim Igoshin <[email protected]>
AuthorDate: Sun Oct 18 07:04:15 2026 +0300
Fix firebird accept data padding (#40188)
* Pad the SRP data of Firebird op_accept_data to XDR alignment
* Add FirebirdPacketPayload#writePadding and use it for op_accept_data
* Use FirebirdPacketPayload#writePadding for the XDR padding of Firebird
packets
* Fix CODE_OF_CONDUCT violations in Firebird padding writes
* Fix CODE_OF_CONDUCT violations in Firebird accept data packet tests
---
.../packet/generic/FirebirdFetchResponsePacket.java | 2 +-
.../packet/generic/FirebirdGenericResponsePacket.java | 2 +-
.../packet/handshake/FirebirdAcceptDataPacket.java | 4 +++-
.../protocol/firebird/payload/FirebirdPacketPayload.java | 15 +++++++++++++--
.../packet/generic/FirebirdFetchResponsePacketTest.java | 6 +++---
.../packet/generic/FirebirdGenericResponsePacketTest.java | 1 +
.../packet/generic/FirebirdSQLResponsePacketTest.java | 4 ++--
.../packet/handshake/FirebirdAcceptDataPacketTest.java | 6 ++++--
.../firebird/payload/FirebirdPacketPayloadTest.java | 10 ++++++++--
9 files changed, 36 insertions(+), 14 deletions(-)
diff --git
a/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdFetchResponsePacket.java
b/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdFetchResponsePacket.java
index 7be84208e68..fd986f6868d 100644
---
a/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdFetchResponsePacket.java
+++
b/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdFetchResponsePacket.java
@@ -55,8 +55,8 @@ public final class FirebirdFetchResponsePacket extends
FirebirdPacket {
}
int nullBitsStartIndex = payload.getByteBuf().writerIndex();
int nullBits = (row.getCells().size() + 7) / 8;
- nullBits += (4 - nullBits) & 3;
payload.getByteBuf().writeZero(nullBits);
+ payload.writePadding(nullBits);
int i = 0;
for (BinaryCell cell : row.getCells()) {
if (null != cell.getData()) {
diff --git
a/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdGenericResponsePacket.java
b/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdGenericResponsePacket.java
index 612c5c0d4fc..f61c2f33629 100644
---
a/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdGenericResponsePacket.java
+++
b/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdGenericResponsePacket.java
@@ -147,7 +147,7 @@ public final class FirebirdGenericResponsePacket extends
FirebirdPacket {
data.write(payload);
int length = payload.getByteBuf().readableBytes() - index;
payload.getByteBuf().setInt(index - 4, length);
- payload.getByteBuf().writeBytes(new byte[(4 - length) & 3]);
+ payload.writePadding(length);
} else {
payload.writeInt4(0);
}
diff --git
a/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/handshake/FirebirdAcceptDataPacket.java
b/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/handshake/FirebirdAcceptDataPacket.java
index cce3b04b569..d7f9c857e25 100644
---
a/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/handshake/FirebirdAcceptDataPacket.java
+++
b/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/packet/handshake/FirebirdAcceptDataPacket.java
@@ -45,11 +45,13 @@ public final class FirebirdAcceptDataPacket extends
FirebirdPacket {
@Override
protected void write(final FirebirdPacketPayload payload) {
if (salt.length != 0 && !publicKey.isEmpty()) {
- payload.writeInt4(salt.length + publicKey.length() + 4);
+ int length = salt.length + publicKey.length() + 4;
+ payload.writeInt4(length);
payload.writeInt2LE(salt.length);
payload.writeBytes(salt);
payload.writeInt2LE(publicKey.length());
payload.writeBytes(publicKey.getBytes(StandardCharsets.US_ASCII));
+ payload.writePadding(length);
} else {
payload.writeInt4(0);
}
diff --git
a/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/payload/FirebirdPacketPayload.java
b/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/payload/FirebirdPacketPayload.java
index 2cc913b3f9d..33ad1d2826e 100644
---
a/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/payload/FirebirdPacketPayload.java
+++
b/database/protocol/dialect/firebird/src/main/java/org/apache/shardingsphere/database/protocol/firebird/payload/FirebirdPacketPayload.java
@@ -22,6 +22,7 @@ import lombok.Getter;
import lombok.RequiredArgsConstructor;
import lombok.Setter;
import org.apache.shardingsphere.database.protocol.payload.PacketPayload;
+import org.apache.shardingsphere.infra.annotation.HighFrequencyInvocation;
import java.nio.charset.Charset;
@@ -209,7 +210,7 @@ public final class FirebirdPacketPayload implements
PacketPayload {
public void writeBuffer(final byte[] value) {
byteBuf.writeInt(value.length);
byteBuf.writeBytes(value);
- byteBuf.writeBytes(new byte[(4 - value.length) & 3]);
+ writePadding(value.length);
}
/**
@@ -220,7 +221,7 @@ public final class FirebirdPacketPayload implements
PacketPayload {
public void writeBuffer(final ByteBuf buffer) {
byteBuf.writeInt(buffer.writerIndex());
byteBuf.writeBytes(buffer);
- byteBuf.writeBytes(new byte[(4 - buffer.writerIndex()) & 3]);
+ writePadding(buffer.writerIndex());
}
/**
@@ -259,6 +260,16 @@ public final class FirebirdPacketPayload implements
PacketPayload {
byteBuf.skipBytes(getPadding(length));
}
+ /**
+ * Write padding to byte buffers.
+ *
+ * @param length length of byte array that needs padding
+ */
+ @HighFrequencyInvocation
+ public void writePadding(final int length) {
+ byteBuf.writeZero(getPadding(length));
+ }
+
/**
* Get padding from byte buffers.
*
diff --git
a/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdFetchResponsePacketTest.java
b/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdFetchResponsePacketTest.java
index e8a372e68e5..cb159d404e3 100644
---
a/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdFetchResponsePacketTest.java
+++
b/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdFetchResponsePacketTest.java
@@ -85,7 +85,6 @@ class FirebirdFetchResponsePacketTest {
void assertWriteWithNonNullCellData() {
when(payload.getByteBuf()).thenReturn(byteBuf);
when(byteBuf.writerIndex()).thenReturn(0);
- when(byteBuf.writeZero(4)).thenReturn(byteBuf);
BinaryRow row = new BinaryRow(Collections.singleton(new
BinaryCell(FirebirdBinaryColumnType.LONG, 123)));
try (MockedStatic<FirebirdBinaryProtocolValueFactory> mocked =
mockStatic(FirebirdBinaryProtocolValueFactory.class)) {
mocked.when(() ->
FirebirdBinaryProtocolValueFactory.getBinaryProtocolValue(FirebirdBinaryColumnType.LONG)).thenReturn(protocolValue);
@@ -93,7 +92,8 @@ class FirebirdFetchResponsePacketTest {
verify(payload).writeInt4(FirebirdCommandPacketType.FETCH_RESPONSE.getValue());
verify(payload).writeInt4(ISCConstants.FETCH_OK);
verify(payload).writeInt4(1);
- verify(byteBuf).writeZero(4);
+ verify(byteBuf).writeZero(1);
+ verify(payload).writePadding(1);
verify(protocolValue).write(payload, 123);
}
}
@@ -114,11 +114,11 @@ class FirebirdFetchResponsePacketTest {
void assertWriteWithNullCellData() {
when(payload.getByteBuf()).thenReturn(byteBuf);
when(byteBuf.writerIndex()).thenReturn(0);
- when(byteBuf.writeZero(4)).thenReturn(byteBuf);
when(byteBuf.getByte(0)).thenReturn((byte) 0);
FirebirdFetchResponsePacket.getFetchRowPacket(new
BinaryRow(Collections.singleton(new BinaryCell(FirebirdBinaryColumnType.LONG,
null)))).write(payload);
verify(payload).writeInt4(FirebirdCommandPacketType.FETCH_RESPONSE.getValue());
verify(payload).writeInt4(ISCConstants.FETCH_OK);
+ verify(payload).writePadding(1);
verify(byteBuf).setByte(0, 1);
}
diff --git
a/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdGenericResponsePacketTest.java
b/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdGenericResponsePacketTest.java
index fd82b9aa490..30dbcd81ecc 100644
---
a/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdGenericResponsePacketTest.java
+++
b/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdGenericResponsePacketTest.java
@@ -133,6 +133,7 @@ class FirebirdGenericResponsePacketTest {
FirebirdPacket data = mock(FirebirdPacket.class);
new
FirebirdGenericResponsePacket().setData(data).setHandle(1).setId(2L).setErrorStatusVector(ex).write(payload);
verify(data).write(payload);
+ verify(payload).writePadding(4);
verify(payload, atLeastOnce()).writeInt4(ISCConstants.isc_arg_gds);
verify(payload, atLeastOnce()).writeInt4(ISCConstants.isc_arg_string);
verify(payload).writeString("foo_error");
diff --git
a/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdSQLResponsePacketTest.java
b/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdSQLResponsePacketTest.java
index 6ff0352b6b5..7c916f848db 100644
---
a/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdSQLResponsePacketTest.java
+++
b/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/generic/FirebirdSQLResponsePacketTest.java
@@ -54,7 +54,6 @@ class FirebirdSQLResponsePacketTest {
void assertWriteWithRow() {
when(payload.getByteBuf()).thenReturn(byteBuf);
when(byteBuf.writerIndex()).thenReturn(0);
- when(byteBuf.writeZero(4)).thenReturn(byteBuf);
BinaryRow row = new BinaryRow(Collections.singleton(new
BinaryCell(FirebirdBinaryColumnType.LONG, 5)));
try (MockedStatic<FirebirdBinaryProtocolValueFactory> mocked =
mockStatic(FirebirdBinaryProtocolValueFactory.class)) {
mocked.when(() ->
FirebirdBinaryProtocolValueFactory.getBinaryProtocolValue(FirebirdBinaryColumnType.LONG)).thenReturn(protocolValue);
@@ -62,7 +61,8 @@ class FirebirdSQLResponsePacketTest {
packet.write(payload);
verify(payload).writeInt4(FirebirdCommandPacketType.SQL_RESPONSE.getValue());
verify(payload).writeInt4(1);
- verify(byteBuf).writeZero(4);
+ verify(byteBuf).writeZero(1);
+ verify(payload).writePadding(1);
verify(protocolValue).write(payload, 5);
}
}
diff --git
a/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/handshake/FirebirdAcceptDataPacketTest.java
b/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/handshake/FirebirdAcceptDataPacketTest.java
index c9357a317d9..1a09473eca9 100644
---
a/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/handshake/FirebirdAcceptDataPacketTest.java
+++
b/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/packet/handshake/FirebirdAcceptDataPacketTest.java
@@ -47,14 +47,15 @@ class FirebirdAcceptDataPacketTest {
packet.write((PacketPayload) payload);
InOrder order = inOrder(payload);
order.verify(payload).writeInt4(expectedPayloadLength);
- verifyPayloadData(order, payload, expectedPayloadData, salt,
publicKey);
+ verifyPayloadData(order, payload, salt, publicKey,
expectedPayloadLength, expectedPayloadData);
order.verify(payload).writeString(plugin.getMethodName());
order.verify(payload).writeInt4(authenticated);
order.verify(payload).writeString(keys);
order.verifyNoMoreInteractions();
}
- private void verifyPayloadData(final InOrder order, final
FirebirdPacketPayload payload, final boolean expectedPayloadData, final byte[]
salt, final String publicKey) {
+ private void verifyPayloadData(final InOrder order, final
FirebirdPacketPayload payload, final byte[] salt, final String publicKey, final
int expectedPayloadLength,
+ final boolean expectedPayloadData) {
if (!expectedPayloadData) {
return;
}
@@ -62,6 +63,7 @@ class FirebirdAcceptDataPacketTest {
order.verify(payload).writeBytes(salt);
order.verify(payload).writeInt2LE(publicKey.length());
order.verify(payload).writeBytes(publicKey.getBytes(StandardCharsets.US_ASCII));
+ order.verify(payload).writePadding(expectedPayloadLength);
}
private static Stream<Arguments> writeArguments() {
diff --git
a/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/payload/FirebirdPacketPayloadTest.java
b/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/payload/FirebirdPacketPayloadTest.java
index aab7b1c6977..b6047c69c49 100644
---
a/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/payload/FirebirdPacketPayloadTest.java
+++
b/database/protocol/dialect/firebird/src/test/java/org/apache/shardingsphere/database/protocol/firebird/payload/FirebirdPacketPayloadTest.java
@@ -148,7 +148,7 @@ class FirebirdPacketPayloadTest {
new FirebirdPacketPayload(byteBuf,
StandardCharsets.UTF_8).writeBuffer(value);
verify(byteBuf).writeInt(4);
verify(byteBuf).writeBytes(value);
- verify(byteBuf).writeBytes(new byte[0]);
+ verify(byteBuf).writeZero(0);
}
@Test
@@ -165,7 +165,7 @@ class FirebirdPacketPayloadTest {
new FirebirdPacketPayload(byteBuf,
StandardCharsets.UTF_8).writeString("value");
verify(byteBuf).writeInt(5);
verify(byteBuf).writeBytes("value".getBytes(StandardCharsets.UTF_8));
- verify(byteBuf).writeBytes(new byte[3]);
+ verify(byteBuf).writeZero(3);
}
@Test
@@ -180,6 +180,12 @@ class FirebirdPacketPayloadTest {
verify(byteBuf).skipBytes(3);
}
+ @Test
+ void assertWritePadding() {
+ new FirebirdPacketPayload(byteBuf,
StandardCharsets.UTF_8).writePadding(5);
+ verify(byteBuf).writeZero(3);
+ }
+
@Test
void assertGetPadding() {
assertThat(new FirebirdPacketPayload(byteBuf,
StandardCharsets.UTF_8).getPadding(5), is(3));
