You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "DrDrunkenstien-10 (via GitHub)" <gi...@apache.org> on 2026/04/21 12:07:06 UTC
[PR] Update StructuredDataMessage Javadoc for RFC 5424 compliance [logging-log4j2]
DrDrunkenstien-10 opened a new pull request, #4103:
URL: https://github.com/apache/logging-log4j2/pull/4103
Fixes #4051
This pull request improves the Javadoc of StructuredDataMessage constructors by clarifying
the expected format and usage of the `id` (SD-ID) and `type` (MSGID) parameters.
Specifically:
- Adds references to RFC 5424 sections for SD-ID and MSGID
- Documents that these parameters are expected to conform to the RFC syntax
- Clarifies that they are treated as trusted inputs (typically compile-time constants)
- Highlights that validation/sanitization is the caller's responsibility when using untrusted input
- Recommends using StructuredDataId instead of raw String where applicable
This change is documentation-only and does not modify runtime behavior.
This improvement is based on feedback from the YesWeHack bug bounty report (#YWH-PGM10209-37),
which identified a lack of clarity in the existing documentation.
## Checklist
* Base your changes on `2.x` branch if you are targeting Log4j 2; use `main` otherwise
- [x] Yes (based on `2.x`)
* `./mvnw verify` succeeds ([the build instructions](https://logging.apache.org/log4j/2.x/development.html#building))
- [x] Yes
* Non-trivial changes contain an entry file in the `src/changelog/.2.x.x` directory
- [ ] Not applicable (documentation-only change; no behavioral impact)
* Tests are provided
- [ ] Not applicable (documentation-only change)
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update StructuredDataMessage Javadoc for RFC 5424 compliance [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on PR #4103:
URL: https://github.com/apache/logging-log4j2/pull/4103#issuecomment-4338260669
@ppkarwasz, would you mind helping with reviewing this one, please?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update StructuredDataMessage Javadoc for RFC 5424 compliance [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #4103:
URL: https://github.com/apache/logging-log4j2/pull/4103#discussion_r3361442844
##########
log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java:
##########
@@ -57,133 +61,363 @@ public enum Format {
}
/**
- * Creates a StructuredDataMessage using an ID (max 32 characters), message, and type (max 32 characters).
- * @param id The String id.
- * @param msg The message.
- * @param type The message type.
+ * Creates a StructuredDataMessage using an SD-ID (max 32 characters), message,
+ * and
+ * MSGID (max 32 characters).
+ * <p>
+ * The {@code sdId} parameter represents the syslog {@code SD-ID} and is
+ * expected
+ * to conform to
+ * <a href="https://datatracker.ietf.org/doc/html/rfc5424#section-6.3.2">RFC
+ * 5424 Section 6.3.2</a>.
+ * It is recommended to use {@link StructuredDataId} instead of a raw
+ * {@link String} where possible,
+ * as it allows specifying a set of allowed keys for structured data elements.
+ * </p>
+ * <p>
+ * The {@code msgId} parameter represents the syslog {@code MSGID} and is
+ * expected to conform to
+ * <a href="https://datatracker.ietf.org/doc/html/rfc5424#section-6.2.7">RFC
+ * 5424 Section 6.2.7</a>.
+ * </p>
+ * <p>
+ * Both {@code sdId} and {@code msgId} are considered trusted inputs (typically
+ * compile-time constants).
+ * If these values are derived from external or untrusted sources, it is the
+ * caller's responsibility
+ * to validate and sanitize them to ensure RFC-compliant output, especially when
+ * used with
+ * {@code Rfc5424Layout}.
+ * </p>
+ *
+ * @param sdId The String SD-ID.
+ * @param msg The message.
+ * @param msgId The identifier MSGID.
*/
- public StructuredDataMessage(final String id, final String msg, final String type) {
- this(id, msg, type, MAX_LENGTH);
+ public StructuredDataMessage(final String sdId, final String msg, final String msgId) {
+ this(sdId, msg, msgId, MAX_LENGTH);
}
/**
- * Creates a StructuredDataMessage using an ID (user specified max characters), message, and type (user specified
+ * Creates a StructuredDataMessage using an SD-ID (user specified max
+ * characters),
+ * message, and MSGID (user specified
* maximum number of characters).
- * @param id The String id.
- * @param msg The message.
- * @param type The message type.
+ * <p>
+ * The {@code sdId} parameter represents the syslog {@code SD-ID} and is
+ * expected
+ * to conform to
+ * <a href="https://datatracker.ietf.org/doc/html/rfc5424#section-6.3.2">RFC
+ * 5424 Section 6.3.2</a>.
+ * It is recommended to use {@link StructuredDataId} instead of a raw
+ * {@link String} where possible,
+ * as it allows specifying a set of allowed keys for structured data elements.
+ * </p>
+ * <p>
+ * The {@code msgId} parameter represents the syslog {@code MSGID} and is
+ * expected to conform to
+ * <a href="https://datatracker.ietf.org/doc/html/rfc5424#section-6.2.7">RFC
+ * 5424 Section 6.2.7</a>.
+ * </p>
+ * <p>
+ * Both {@code sdId} and {@code msgId} are considered trusted inputs (typically
+ * compile-time constants).
+ * If these values are derived from external or untrusted sources, it is the
+ * caller's responsibility
+ * to validate and sanitize them to ensure RFC-compliant output, especially when
+ * used with
+ * {@code Rfc5424Layout}.
+ * </p>
+ *
+ * @param sdId The String SD-ID.
+ * @param msg The message.
+ * @param msgId The message identifier MSGID.
* @param maxLength The maximum length of keys;
* @since 2.9.0
*/
- public StructuredDataMessage(final String id, final String msg, final String type, final int maxLength) {
- this.id = new StructuredDataId(id, null, null, maxLength);
+ public StructuredDataMessage(final String sdId, final String msg, final String msgId, final int maxLength) {
+ validateSdId(sdId);
+ validateMsgId(msgId);
+
+ this.sdId = new StructuredDataId(sdId, null, null, maxLength);
Review Comment:
Doesn't `StructuredDataId::new` need to have some validation too?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update StructuredDataMessage Javadoc for RFC 5424 compliance [logging-log4j2]
Posted by "DrDrunkenstien-10 (via GitHub)" <gi...@apache.org>.
DrDrunkenstien-10 commented on PR #4103:
URL: https://github.com/apache/logging-log4j2/pull/4103#issuecomment-4540095049
@ramanathan1504 Thank You for the feedback!
I'll work on updating the PR to incorporate these changes.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update StructuredDataMessage Javadoc for RFC 5424 compliance [logging-log4j2]
Posted by "DrDrunkenstien-10 (via GitHub)" <gi...@apache.org>.
DrDrunkenstien-10 commented on PR #4103:
URL: https://github.com/apache/logging-log4j2/pull/4103#issuecomment-4428138043
@ppkarwasz
Updated the PR to validate invalid SD-ID and MSGID inputs, renamed `id` to `sdId` and `type` to `msgId` in both the code and Javadocs, and updated the Javadocs to mention that `StructuredDataId` is recommended because it allows specifying allowed keys.
I did not implement enforcement of the allowed keys defined by `StructuredDataId`, since that appears to be a separate behavioral issue and may be better addressed in its own PR. I'm happy to include that change in this PR as well if you would prefer.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update StructuredDataMessage Javadoc for RFC 5424 compliance [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on PR #4103:
URL: https://github.com/apache/logging-log4j2/pull/4103#issuecomment-5713979400
@vy sorry! mistakenly pushed in single commit. I wont repeat this again. Thanks!
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update StructuredDataMessage Javadoc for RFC 5424 compliance [logging-log4j2]
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on code in PR #4103:
URL: https://github.com/apache/logging-log4j2/pull/4103#discussion_r3361444435
##########
log4j-api/src/main/java/org/apache/logging/log4j/message/StructuredDataMessage.java:
##########
@@ -57,133 +61,363 @@ public enum Format {
}
/**
- * Creates a StructuredDataMessage using an ID (max 32 characters), message, and type (max 32 characters).
- * @param id The String id.
- * @param msg The message.
- * @param type The message type.
+ * Creates a StructuredDataMessage using an SD-ID (max 32 characters), message,
+ * and
+ * MSGID (max 32 characters).
+ * <p>
+ * The {@code sdId} parameter represents the syslog {@code SD-ID} and is
+ * expected
+ * to conform to
+ * <a href="https://datatracker.ietf.org/doc/html/rfc5424#section-6.3.2">RFC
+ * 5424 Section 6.3.2</a>.
+ * It is recommended to use {@link StructuredDataId} instead of a raw
+ * {@link String} where possible,
+ * as it allows specifying a set of allowed keys for structured data elements.
+ * </p>
+ * <p>
+ * The {@code msgId} parameter represents the syslog {@code MSGID} and is
+ * expected to conform to
+ * <a href="https://datatracker.ietf.org/doc/html/rfc5424#section-6.2.7">RFC
+ * 5424 Section 6.2.7</a>.
+ * </p>
+ * <p>
+ * Both {@code sdId} and {@code msgId} are considered trusted inputs (typically
+ * compile-time constants).
+ * If these values are derived from external or untrusted sources, it is the
+ * caller's responsibility
+ * to validate and sanitize them to ensure RFC-compliant output, especially when
+ * used with
+ * {@code Rfc5424Layout}.
+ * </p>
+ *
+ * @param sdId The String SD-ID.
+ * @param msg The message.
+ * @param msgId The identifier MSGID.
*/
- public StructuredDataMessage(final String id, final String msg, final String type) {
- this(id, msg, type, MAX_LENGTH);
+ public StructuredDataMessage(final String sdId, final String msg, final String msgId) {
+ this(sdId, msg, msgId, MAX_LENGTH);
}
/**
- * Creates a StructuredDataMessage using an ID (user specified max characters), message, and type (user specified
+ * Creates a StructuredDataMessage using an SD-ID (user specified max
+ * characters),
+ * message, and MSGID (user specified
* maximum number of characters).
- * @param id The String id.
- * @param msg The message.
- * @param type The message type.
+ * <p>
+ * The {@code sdId} parameter represents the syslog {@code SD-ID} and is
+ * expected
+ * to conform to
+ * <a href="https://datatracker.ietf.org/doc/html/rfc5424#section-6.3.2">RFC
+ * 5424 Section 6.3.2</a>.
+ * It is recommended to use {@link StructuredDataId} instead of a raw
+ * {@link String} where possible,
+ * as it allows specifying a set of allowed keys for structured data elements.
+ * </p>
+ * <p>
+ * The {@code msgId} parameter represents the syslog {@code MSGID} and is
+ * expected to conform to
+ * <a href="https://datatracker.ietf.org/doc/html/rfc5424#section-6.2.7">RFC
+ * 5424 Section 6.2.7</a>.
+ * </p>
+ * <p>
+ * Both {@code sdId} and {@code msgId} are considered trusted inputs (typically
+ * compile-time constants).
+ * If these values are derived from external or untrusted sources, it is the
+ * caller's responsibility
+ * to validate and sanitize them to ensure RFC-compliant output, especially when
+ * used with
+ * {@code Rfc5424Layout}.
+ * </p>
+ *
+ * @param sdId The String SD-ID.
+ * @param msg The message.
+ * @param msgId The message identifier MSGID.
* @param maxLength The maximum length of keys;
* @since 2.9.0
*/
- public StructuredDataMessage(final String id, final String msg, final String type, final int maxLength) {
- this.id = new StructuredDataId(id, null, null, maxLength);
+ public StructuredDataMessage(final String sdId, final String msg, final String msgId, final int maxLength) {
Review Comment:
Can we have one canonical constructor and make the rest of the constructors to delegate to that, please?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update StructuredDataMessage Javadoc for RFC 5424 compliance [logging-log4j2]
Posted by "ramanathan1504 (via GitHub)" <gi...@apache.org>.
ramanathan1504 commented on PR #4103:
URL: https://github.com/apache/logging-log4j2/pull/4103#issuecomment-4499337496
Hi @DrDrunkenstien-10!
Appreciate your patience!
This is a really fantastic PR. Not only does it fix the documentation as requested in Issue #4051, but you also made a great catch by adding the fail-fast validation directly into the constructors to strictly enforce RFC 5424 compliance.
Before we merge this, there are just a few small things we need to wrap up:
1. **Unit Tests:** Since this PR introduces new validation logic that actively throws `IllegalArgumentException` in the constructors, could you please add a few unit tests to `StructuredDataMessageTest.java`?
* It would be great to have a "happy path" test (e.g., exactly 32 valid characters).
* A few "failure path" tests verifying that `IllegalArgumentException` is correctly thrown for inputs that are null, exceed 32 characters, or contain forbidden characters (like spaces, `=`, `]`, or `"`).
2. **Minor Typo:** In the `validateKey` method, there is a missing space in the exception message string concatenation. It currently reads `"characters" + "and"`, which will print as `charactersand`. Adding a space there would be perfect!
3. **Changelog Entry:** Could you please add a quick changelog entry for this update? (If you haven't done this before, Log4j2 uses a specific folder structure for changelogs, usually under `src/changelog/`—let us know if you need any guidance on how to generate or format this!).
Thank you again for this excellent contribution. Please feel free to ping here if you have any questions or need help setting up the tests or changelog!
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update StructuredDataMessage Javadoc for RFC 5424 compliance [logging-log4j2]
Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on PR #4103:
URL: https://github.com/apache/logging-log4j2/pull/4103#issuecomment-4338457806
Hi @DrDrunkenstien-10,
Thank you for the contribution, the Javadoc improvement is very appreciated! :heart:
However, since we'll be preparing a new minor version soon, I think we can push this further: throw an `IllegalArgumentException`, whenever a user passes an invalid argument. This is already true, whenever a user calls one of the `with(...)` methods on this class: the key is checked for compliance with RFC5424. We could have exactly the same behavior for `id` and `type` (BTW: it would be useful to rename them to `sdId` and `msgId`).
Regarding the recommendation of using `StructuredDataId`, we should add a reason:
> `StructuredDataId` allows you to specify the list of allowed keys
While the above is true, there seems to be another small problem in the code: the possible fields specified by `StructuredDataId` are never actually checked. :wink:
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update StructuredDataMessage Javadoc for RFC 5424 compliance [logging-log4j2]
Posted by "DrDrunkenstien-10 (via GitHub)" <gi...@apache.org>.
DrDrunkenstien-10 commented on PR #4103:
URL: https://github.com/apache/logging-log4j2/pull/4103#issuecomment-4368737556
@ppkarwasz Thank you for the feedback!
I’ll work on updating the PR to incorporate these changes.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org