Repository navigation
HttpMaxFrameSizeTest.sendOversizedMessageTest() passes silently when maxFrameSize is not enforced #2706
Description
Component: activemq-http (test)
Version: present on main as of 2026-10-19, commit 644cf4fd
Description
HttpMaxFrameSizeTest.sendOversizedMessageTest() sends a 5000-byte message to a connector configured with maxFrameSize=4000 and asserts on the resulting JMSException, but nothing fails the test when no exception is thrown:
try {
send(5000);
} catch (JMSException jmsException) {
Assert.assertTrue(jmsException.getMessage().contains("405 Method Not Allowed"));
}
If the frame-size limit stops being enforced, send(5000) returns normally, the
catch block never runs, and the test passes. The assertion inside the catch is the
only check in the method.
Impact
Test-only: no runtime effect. The cost is lost regression coverage; this test is what guards the behaviour added in AMQ-7327 and refined in AMQ-7419 (405 instead of 500 on oversized frames), and in its current form it cannot fail when that behaviour regresses.
Proposed fix
public void sendOversizedMessageTest() throws Exception {
try {
send(5000);
+ Assert.fail("Expected JMSException: maxFrameSize should have been enforced");
} catch (JMSException jmsException) {
Assert.assertTrue(jmsException.getMessage().contains("405 Method Not Allowed"));
}
}
I searched the open and closed issues and found no existing report. Happy to open a PR with the fix if it is welcomed.
Activity
dimitribekale commentedon Oct 20, 2026
As disclosed in #2623, this was found during an academic study of LLM-assisted code review. An automated LLM-based reviewer produced the initial analysis, and I verified it by reading the source at 644cf4f.
Chavva-Harshita commentedon Nov 2, 2026
Hi! I'd be interested in taking this issue up and contributing a PR.
I went through the test and agree that the missing failure assertion means the test can pass even when the frame-size limit isn't enforced.
I'd like to implement the proposed
Assert.fail()check, verify the behaviour with the existing test suite, and submit a PR.Would it be okay if I take this up?
dimitribekale commentedon Nov 5, 2026
@Chavva-Harshita Please go ahead, that would be great. The proposed check is the
Assert.fail()after the send in the test, as described above; if you want the exact lines I used when verifying, say so and I will paste them.Chavva-Harshita commentedon Nov 6, 2026
Thank you!
I'll go ahead with the implementation and will submit the PR soon
Please share the exact lines you used while verifying the issue. I'll use them to validate the fix, run the relevant existing test suite, and ensure the assertion behaves as expected before submitting the PR.
Chavva-Harshita commentedon Nov 10, 2026
Hi @dimitribekale ! I've successfully submitted the PR #2831 with the proposed changes.
Thank you for your guidance and for giving me the opportunity to work on this issue. I appreciate it!