Bug 995079 - Delivery properties "expiration" and "timestamp" are in milliseconds instead of seconds
Delivery properties "expiration" and "timestamp" are in milliseconds instead ...
Status: NEW
Product: Red Hat Enterprise MRG
Classification: Red Hat
Component: qpid-java (Show other bugs)
2.3
All All
medium Severity medium
: ---
: ---
Assigned To: messaging-bugs
Messaging QE
: EasyFix, Patch, TestCaseProvided
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-08 09:56 EDT by Pavel Moravec
Modified: 2015-10-13 09:10 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Simple patch (2.17 KB, patch)
2013-08-14 07:06 EDT, Pavel Moravec
no flags Details | Diff
reproducer (2.08 KB, text/x-java)
2013-08-14 07:16 EDT, Pavel Moravec
no flags Details
Trivial patch proposal (1.89 KB, patch)
2013-08-15 04:07 EDT, Pavel Moravec
no flags Details | Diff


External Trackers
Tracker ID Priority Status Summary Last Updated
Apache JIRA QPID-5057 None None None Never

  None (edit)
Description Pavel Moravec 2013-08-08 09:56:43 EDT
Description of problem:


Per AMQP 0.10 specification, message delivery properties "expiration" and "timestamp" are of type datetime, that is 64 bit POSIX time_t format, i.e. "seconds since Epoch".

But Java client implementation of both 0.8 and 0.10 protocol versions encodes the properties in milliseconds. See e.g. org/apache/qpid/client/BasicMessageProducer_0_10.java:

long currentTime = 0;
if (timeToLive > 0 || !isDisableTimestamps())
{ currentTime = System.currentTimeMillis(); }

if (timeToLive > 0)
{ deliveryProp.setTtl(timeToLive); message.setJMSExpiration(currentTime + timeToLive); }

if (!isDisableTimestamps())
{ deliveryProp.setTimestamp(currentTime); message.setJMSTimestamp(currentTime); }

I.e. there should be "currentTime = System.currentTimeMillis()/1000;", rather.

The same is in 0.8 client as well (while AMQP 0.8 specification does not know either of the two delivery properties).

I could propose a trivial patch for the 0.8 and 0.10 client, but I am not sure how this affects:

- AMQP 1.0 ongoing implementation I am not familiar with
- (Java only?) broker or other clients utilizing the properties


Version-Release number of selected component (if applicable):
any (i.e. 0.18-7)


How reproducible:
100%


Steps to Reproduce:
1. Send various messages in Java client:
  - with everything default
  - with setting JMS timestamp (msg3.setJMSTimestamp(1);)
  - with setting JMS expiration (msg1.setJMSExpiration(System.currentTimeMillis()/1000+100)
  - maybe some other examples I am not aware of now
2. Check e.g. in tcpdump message headers "timestamp" and "expiration" 


Actual results:
- "timestamp" is everytime set to too far future like "Mar 9,  45572"
- "expiration", until specifically set by setJMSExpiration, is also in far future


Expected results:
- current date and time seen rather


Additional info:
Comment 1 Pavel Moravec 2013-08-14 07:06:28 EDT
Created attachment 786500 [details]
Simple patch

Patch proposal fixing two bugs, in fact:

1) expiration and timestamp delivery properties sent in proper format (datetime in seconds, not in ms).

2) ability to re-write timestamp (i.e. set timestamp automatically only if it hasn't been already set)

Reproducer scenario for 2): send a message with:

msg3.setJMSTimestamp(System.currentTimeMillis()/1000+100);

and check it will NOT have timestamp 100 seconds in advance.
Comment 2 Pavel Moravec 2013-08-14 07:16:33 EDT
Created attachment 786504 [details]
reproducer

Reproducer that sends 4 messages to DurableQueue, one after another having:
- expiration set 10 seconds in advance (this wont cause the message to expire though, see AMQP 0.10 specification for reasoning)
- default one
- timestamp set 100 seconds in advance
- TTL set to 10 seconds

See e.g. in tcpdump what is really sent.

Current behaviour:
- 1st message has proper timestamp in year 45588 or so and expiration=currentTime+10
- 2nd message has timestamp in year 45588 or so
- 3rd message has timestamp in year 45588 or so
- 4th message has timestamp in year 45588 or so,  expiration=timestamp+10 (also too far in future) and TTL=10

Expected behaviour:
- 1st message has proper timestamp=currentTime and expiration=currentTime+10
- 2nd message has timestamp=currentTime
- 3rd message has timestamp=currentTime+100
- 4th message has timestamp=currentTime, expiration=currentTime+10 and TTL=10
Comment 3 Pavel Moravec 2013-08-15 04:07:59 EDT
Created attachment 786858 [details]
Trivial patch proposal

This patch fixes just the format, and does not allow re-writing timestamp.

The timestamp setting would be another issue and moreover not a valid one. As quoting JMS spec for setJMSTimestamp (http://docs.oracle.com/javaee/6/api/javax/jms/Message.html#setJMSTimestamp%28long%29): "This method can be used to change the value for a message that has been received." So no word about senders.

I checked neither expiration or timestamp is mentioned in consumers in Java client, hence no reverse conversion is required to be added to the patch.

From the reproducer, 3rd message should be ignored (it tests setting timestamp).
Comment 4 Robbie Gemmell 2014-09-09 09:46:22 EDT
As per the comments on https://issues.apache.org/jira/browse/QPID-5057:

There would be additional changes required to complete this, and resulting client and [Java] broker version compatibility issues to take into account from making such a change.

Both myself and Gordon felt that 0-10 spec compliance was not in itself sufficient reason to alter the existing behaviour, instead requiring some more significant driver.

Note You need to log in before you can comment on or make changes to this bug.