Introduce try/catch for image creation #2717
Conversation
As reported in #2682, invalid images (but valid a valid file) would report a 500 error when CDash attempts to create the image from the information. Catch the error on creation and return early, as if the image had an unknown extension. Set the checksum to 0 to avoid writing other error messages to the log
| $img = imagecreatefromstring($imgStr); | ||
| } catch (ErrorException) { | ||
| Log::error("Unable to create image object from data in #{$this->testName}"); | ||
| $image->Checksum = 0; |
There was a problem hiding this comment.
What is the purpose of this line?
There was a problem hiding this comment.
@williamjallen, without setting any value for $image->Checksum an additional error message was generated:
[2025-02-14 18:12:11] testing.ERROR: Unable to create image object from data in #my_test {"projectid":9,"buildid":59}
[2025-02-14 18:12:11] testing.ERROR: ERROR: null value in column "checksum" of relation "image" violates not-null constraint
DETAIL: Failing row contains (3, \x633256304b454e5552564e5558314e5056564a445256394553564a46513152..., image/png, null). {"projectid":9,"buildid":59}
I assumed that adding a zero file, rather than denying the entry entirely, would allow the comparison to show that one of them was invalid, as as shown below
There was a problem hiding this comment.
I don't like the idea of storing "dummy" data in place of something which wasn't valid to begin with. If a submission is not valid, why shouldn't we just reject it entirely?
There was a problem hiding this comment.
@williamjallen, I agree to an extent. There are times where an invalid submission should be wholly rejected, I think that a valid test outcome of an invalid image being sent up for comparison should not be grounds for rejection
There was a problem hiding this comment.
Hmm, if we don't reject the entire submission, what about just not saving the image in the first place? I'm generally a fan of fail-fast systems, but I agree that there could be a place for handling the error instead here.
My primary concern with not failing the submission is that if we don't cause the submission to fail, how will anyone know that the "image" they're submitting is actually a text file?
As reported in #2682, invalid images (when supplied a valid path to a non-image) would report a 500 error when CDash attempts to create the image from the information. Catch the error on creation and return early, as if the image had an unknown extension.
Set the checksum to 0 to avoid writing other error messages to the log