Update StructuredDataMessage Javadoc for RFC 5424 compliance

[DrDrunkenstien-10]

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

  • Yes (based on 2.x)

• ./mvnw verify succeeds (the build instructions)

  • 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)

[vy]

@ppkarwasz, would you mind helping with reviewing this one, please?

[ppkarwasz]

Hi @DrDrunkenstien-10,

Thank you for the contribution, the Javadoc improvement is very appreciated! ❤️

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. 😉

[DrDrunkenstien-10]

@ppkarwasz Thank you for the feedback!

I’ll work on updating the PR to incorporate these changes.

[DrDrunkenstien-10]

@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.

[ramanathan1504]

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. 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!

Note: Avoid force push

[DrDrunkenstien-10]

@ramanathan1504 Thank you for the feedback!

I'll work on updating the PR to incorporate these changes.

[vy]

Doesn't StructuredDataId::new need to have some validation too?

[vy]

Can we have one canonical constructor and make the rest of the constructors to delegate to that, please?