Comment 18 for bug 1940141

Revision history for this message
Bruce Elrick (virtuous-sloth) wrote :

Good point, Dan. A caveat to my work here on this patch: I did some C programming and played with the Perl test harness in the early 90s which gave me enough abililty to develop this patch set but I would benefit from any deeper knowledge someone else may have, especially anyone with openssl knowledge.

In terms of due dilligance, I've searched through the source files for references to 'TSProxy' which is the parent directory for a number of the test hardness perl modules changes in the commit and nothing I find indicates it is referenced by the actual openssl C code but only is part of the build/test process.

For extra due diligence I built with only the tests change backported and two tests fail. Tests failing is expected because the actual functional backport is missing. When I add the functional backport then all tests succeed, as expected. When I then make the test optional, the same two tests fail, as expected. See the test-results.txt attachment for more.

I fix the tests by enabling the fix for 'Test 6' only in openssl-1.1.1/test/recipes/70-test_tls13messages.t by wrapping the test with perl code to set and delete the FIX_LP_1940141 environment variable. As an aside, I now see that other such env var setting does not use single-quoted strings but rather just bare strings; this is allowed in perl and my patch does not followed the style in the rest of the test file.

My understanding is that there are several flags for subtests, thus accounting for the two passing or failing.

But more to your point about the risk of backporting the tests, my assessment
is that the perl modules are only in testing and while there are a non-trivial
number of tests added along with the one that tests what is being backported,
the fact that they all pass suggests that they are relevant here.

That is, it looks to me these additional tests are not-inappropriate, are
actually appropriate, and in fact the reason they were added later was this
fix forced the issue on the lack of capability in the test coverage.