Skip to content

Enforce provider-only JMSX properties under strictCompliance#1853

Open
pradeep85841 wants to merge 2 commits intoapache:mainfrom
pradeep85841:strict-compliance-provider-jmsx
Open

Enforce provider-only JMSX properties under strictCompliance#1853
pradeep85841 wants to merge 2 commits intoapache:mainfrom
pradeep85841:strict-compliance-provider-jmsx

Conversation

@pradeep85841
Copy link
Copy Markdown
Contributor

  • Throws a MessageNotWriteableException if a client attempts to set JMSXDeliveryCount, JMSXRcvTimestamp, JMSXState, JMSXProducerTXID, or JMSXConsumerTXID.
  • Client-set properties (like JMSXGroupID and JMSXGroupSeq) are completely bypassed and function normally.
  • Added StrictComplianceProviderJMSXPropertyTest using an embedded BrokerService to explicitly verify both strict and legacy behaviors.

Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE)) {

connection.start();
Message message = (Message) session.createMessage();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see Message in the import, I think it won't compile 😄

"JMSXState".equals(name) ||
"JMSXProducerTXID".equals(name) ||
"JMSXConsumerTXID".equals(name)) {
throw new MessageNotWriteableException("Provider-set JMSX property '" + name + "' cannot be set by a client under strict compliance.");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should use MessageNotWriteableException here.

MessageNotWriiteableException is specified in JMS for when a message is in read-only mode (received). For a provider-only property that clients should never set, I think a JMSException with a clear message might be more semantically accurate.

The distinction matters because a client catching MessageNotWriteableException might assume calling clearProperties() would fix it, but it won't (since this is a permanent restriction, not a state-based one).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants