message_parse_received_date() avoid calling message_parse_string(hdr="")

[dilyanpalauzov]

as the latter does hdr = strchr(hdr+1, '\n') and hdr+1 is not allocated.

Without the change in message.c, calling

$ valgrind --trace-children=yes ./cunit/unit cunit/message:parse_received_semicolon

prints

==176956== Invalid read of size 1                                   
==176956==    at 0x7F25960: __strchr_avx2 (strchr-avx2.S:53)        
==176956==    by 0x490AA62: message_parse_string (message.c:1180)                                                                       
==176956==    by 0x490CCDE: message_parse_received_date (message.c:2019)         
==176956==    by 0x490A3F1: message_parse_headers (message.c:981)                                                                       
==176956==    by 0x4909BAD: message_parse_body (message.c:779)                                                                          
==176956==    by 0x4908FEF: message_parse_mapped (message.c:525)    
==176956==    by 0x53800A: test_parse_received_semicolon (message.testc:1403)
==176956==    by 0x4112F8: __cunit_wrap_test (unit.c:159)
==176956==    by 0x538C5E: __cunit_test_parse_received_semicolon (message.testc-cunit.c:143)
==176956==    by 0x789266A: run_single_test.constprop.0 (TestRun.c:991)
==176956==    by 0x7894DE7: CU_run_test (TestRun.c:473)
==176956==    by 0x411C25: run_tests (unit.c:389)
==176956==  Address 0x8563245 is 0 bytes after a block of size 5 alloc'd
==176956==    at 0x484278B: malloc (vg_replace_malloc.c:431)
==176956==    by 0x4C4E328: xmalloc (xmalloc.c:56)
==176956==    by 0x4C4E553: xstrndup (xmalloc.c:115)
==176956==    by 0x490AAE3: message_parse_string (message.c:1190)
==176956==    by 0x490CC78: message_parse_received_date (message.c:1999)
==176956==    by 0x490A3F1: message_parse_headers (message.c:981)
==176956==    by 0x4909BAD: message_parse_body (message.c:779)
==176956==    by 0x4908FEF: message_parse_mapped (message.c:525)
==176956==    by 0x53800A: test_parse_received_semicolon (message.testc:1403)
==176956==    by 0x4112F8: __cunit_wrap_test (unit.c:159)
==176956==    by 0x538C5E: __cunit_test_parse_received_semicolon (message.testc-cunit.c:143)
==176956==    by 0x789266A: run_single_test.constprop.0 (TestRun.c:991)

[elliefm]

The fix looks good, and the test proves that it fixes the invalid read. But the test itself fails... looks like a simple fix though, see suggestion inline.

[elliefm]

The test fails here with the fix, because when a semicolon with nothing after it is found, the original header value is used as the date string, which in this case is "abc;". But this line expects the empty string. It should expect "abc;", like this:

Suggested change

	CU_ASSERT_STRING_EQUAL(body.received_date, "");
	CU_ASSERT_STRING_EQUAL(body.received_date, "abc;");

[rsto]

Neither "abc" nor "" looks right to me. A non-existent date-time part in the header value should result in a NULL received_date value. The code building on body->received_date in lmtpd and jmap_mail is already prepared to handle NULL received_dates.

[rsto]

Thanks for submitting this. I will accept this as-is but will rewrite setting received_date to NULL on top of it. I'll then merge the updated PR so that CI succeeds.

[elliefm]

@elliefm When backporting this, see if you can integrate the Received header test changes from #4776. That test should work just as well with this version of the implementation with probably the same tweaks as my suggestion in this PR.