Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DefaultMessagePropertiesConverter#toMessageProperties should handle x-delay in Short #2667

Closed
seanliu-oss opened this issue Mar 27, 2024 · 4 comments

Comments

@seanliu-oss
Copy link
Contributor

seanliu-oss commented Mar 27, 2024

In what version(s) of Spring AMQP are you seeing this issue?
3.0.4
image

Describe the bug
When sending a message via delayed-message-exchange, and get the message via amqpTemplate, we get a null receivedDelay

To Reproduce
Send a message via delayed-message-exchange, with a delay of 5 seconds. The message will show up with x-delay of -5000 in the queue:
image

When using amqpTemplate to receive the message, it reaches DefaultMessagePropertiesConverter#toMessageProperties, the header value shows up as type Short:
image
image

Therefore the message will show up with receivedDelay as null.
image

While we are at it, we should also ensure receivedDelay is a positive number here, therefore a Math.abs() should help.
I did observe negative receivedDelay, but have not been able to reproduce it yet.

Expected behavior

The message should show up with receivedDelay as 5000

Sample

@SpringBootApplication
public class RabbitTestApplication implements CommandLineRunner {
@Autowired
private AmqpTemplate amqpTemplate;

public static void main(String[] args) {

    SpringApplication.run(BunyipRabbitTestApplication.class, args);
}

@Override
public void run(String... args) throws Exception {
    Message message = amqpTemplate.receive("test-delay");
}

}

@artembilan
Copy link
Member

The -5000 is even explained in the respective RabbitMQ docs: https://www.rabbitmq.com/blog/2015/04/16/scheduling-messages-with-rabbitmq#checking-if-a-message-was-delayed.
They also mention that this property is an integer.
Plus it is not clear how that can be a short: #2602.

We probably can go ahead and try to resolve any Number impl over there into an expected long.

What RabbitMQ broker version do you use?

@seanliu-oss
Copy link
Contributor Author

seanliu-oss commented Mar 27, 2024

I am using 3.12.12 broker:
image

I understand the where -5000 is from on the queue, but it looks like when client retrieves it, it provides back a positive number, I am not sure how the spec says about this behavior.
When I trace into ContentHeaderPropertyReader#readTable, the input stream's buffer has following values:
[0, 60, 0, 0, 0, 0, 0, 0, 0, 0, 0, 15, -96, -64, 10, 116, 101, 120, 116, 47, 112, 108, 97, 105, 110, 0, 0, 0, 11, 7, 120, 45, 100, 101, 108, 97, 121, 115, 19, -120, 36, 48, 56, 54, 54, 53, 102, 52, 55, 45, 100, 100, 99, 48, 45, 52, 55, 101, 56, 45, 57, 54, 98, 56, 45, 56, 51, 56, 51, 53, 51, 100, 100, 57, 54, 49, 98, 0, 0, 0, 0, 102, 4, 104, -29]
If I print it out, I get 13, -78 after x-delay, which is 0x1388, and translate into 5000 . I am not sure why in this case in specs, but it seems like it.

`
0x0
< 0x3c
0x0
0x0
0x0
0x0
0x0
0x0
0x0
0x0
0x0
0xf
0xa0
0xc0

0xa

t 0x74
e 0x65
x 0x78
t 0x74
/ 0x2f
p 0x70
l 0x6c
a 0x61
i 0x69
n 0x6e
0x0
0x0
0x0
� 0xb
0x7
x 0x78

  • 0x2d
    d 0x64
    e 0x65
    l 0x6c
    a 0x61
    y 0x79
    s 0x73
    0x13
    0x88
    $ 0x24
    d 0x64
    7 0x37
    0 0x30
    d 0x64
    2 0x32
    b 0x62
    6 0x36
    e 0x65
  • 0x2d
    9 0x39
    9 0x39
    8 0x38
    9 0x39
  • 0x2d
    4 0x34
    a 0x61
    9 0x39
    9 0x39
  • 0x2d
    8 0x38
    9 0x39
    f 0x66
    3 0x33
  • 0x2d
    c 0x63
    e 0x65
    4 0x34
    6 0x36
    c 0x63
    d 0x64
    c 0x63
    3 0x33
    a 0x61
    9 0x39
    3 0x33
    b 0x62
    0x0
    0x0
    0x0
    0x0
    f 0x66
    0x4
    0x7f
    0xd4
    `

@artembilan
Copy link
Member

Yeah... I'm not very good in parsing HEX, but if whatever I suggested before about Number interaction in the DefaultMessagePropertiesConverter makes sense for you, I'll be more than happy to provide that fix.

Thanks

@seanliu-oss
Copy link
Contributor Author

That would be great, thanks.

@artembilan artembilan added this to the 3.1.4 milestone Mar 27, 2024
artembilan added a commit that referenced this issue Mar 28, 2024
Fixes: #2667

The `x-delay` header may arrive as a `Short` from RabbitMQ broker.

* Fix `DefaultMessagePropertiesConverter.toMessageProperties()` to deal with a `Number`
to extract `long` value from the `x-delay` header

# Conflicts:
#	spring-rabbit/src/main/java/org/springframework/amqp/rabbit/support/DefaultMessagePropertiesConverter.java
artembilan pushed a commit that referenced this issue Apr 1, 2024
Related to #2667

The `DefaultMessagePropertiesConverter.toMessageProperties()` may receive an `x-delay` header as negative value.

**Auto-cherry-pick to `3.0.x`**
artembilan pushed a commit that referenced this issue Apr 1, 2024
Related to #2667

The `DefaultMessagePropertiesConverter.toMessageProperties()` may receive an `x-delay` header as negative value.

# Conflicts:
#	spring-rabbit/src/main/java/org/springframework/amqp/rabbit/support/DefaultMessagePropertiesConverter.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants