davsclaus commented on code in PR #28846:
URL: https://github.com/apache/camel/pull/28846#discussion_r4440716523
##########
components/camel-syslog/src/generated/java/org/apache/camel/component/syslog/SyslogDataFormatConfigurer.java:
##########
@@ -2,7 +2,6 @@
package org.apache.camel.component.syslog;
import javax.annotation.processing.Generated;
-import java.util.Locale;
Review Comment:
This reverts generated output from main: since CAMEL-26439 (#28775,
`property-configurer.vm`) every configurer imports `java.util.Locale`. This was
probably generated with an older `camel-package-maven-plugin` from the local
repo. CI regenerates this file and will fail on the uncommitted diff, so please
drop the file from the PR (`git checkout origin/main -- <this file>`).
##########
components/camel-syslog/src/main/java/org/apache/camel/component/syslog/netty/Rfc5425FrameDecoder.java:
##########
@@ -33,33 +32,29 @@ public class Rfc5425FrameDecoder extends
ByteToMessageDecoder implements Channel
protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object>
out) throws Exception {
if (currentFramelength == null) {
// find index of the first space, it should be after the length
field
- int index = indexOf(in, Unpooled.wrappedBuffer(new byte[] { ' '
}));
-
- // Read part until the first space, if we have found one
- StringBuilder lengthbuffer = new StringBuilder();
- if (index > -1) {
- ByteBuf byteBuf = in.readBytes(index);
- byte[] dest = new byte[byteBuf.readableBytes()];
- byteBuf.readBytes(dest);
- lengthbuffer.append(new String(dest));
+ int spaceIndex = in.indexOf(in.readerIndex(), in.writerIndex(),
(byte) ' ');
+ if (spaceIndex < 0) {
+ // The length delimiter has not arrived yet, wait for more
data without modifying reader index
+ return;
}
- int length;
- try {
- // add one because we have to take in account the space after
- // the length field
- length = Integer.parseInt(lengthbuffer.toString()) + 1;
- } catch (NumberFormatException e) {
- length = -1;
+ int lengthDigits = spaceIndex - in.readerIndex();
+ if (lengthDigits == 0) {
+ throw new IllegalArgumentException("Invalid RFC 5425 frame:
empty length field");
}
- // We have not found the length field, reset the buffer so we can
- // retry next time
- if (length < 0) {
- in.resetReaderIndex();
- return;
+ int length = 0;
+ for (int i = 0; i < lengthDigits; i++) {
+ byte b = in.getByte(in.readerIndex() + i);
+ if (b < '0' || b > '9') {
+ throw new IllegalArgumentException("Invalid RFC 5425
frame: non-digit character in length field");
+ }
+ length = length * 10 + (b - '0');
Review Comment:
Overflow: `Integer.parseInt` used to reject a length field that does not fit
an int, but this loop wraps it. For example, `4294967296 ` becomes length 0 and
the decoder silently mis-frames the rest of the stream. `2147483647 ` makes
`length + 1` negative, so `readBytes` fails later with a less clear error.
Also, a peer that sends digits without a space makes the decoder buffer
forever, because the `indexOf` search has no bound.
RFC 5425 frames are bounded in practice, so capping the length field fixes
both problems: reject it when it has more than 9 digits, or when no space shows
up within the first 10 readable bytes. For example:
```java
int spaceIndex = in.indexOf(in.readerIndex(), Math.min(in.writerIndex(),
in.readerIndex() + MAX_LENGTH_DIGITS + 1), (byte) ' ');
if (spaceIndex < 0) {
if (in.readableBytes() > MAX_LENGTH_DIGITS) {
throw new IllegalArgumentException("Invalid RFC 5425 frame: length
field too long");
}
return;
}
```
with `MAX_LENGTH_DIGITS = 9`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
