Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 354192

Summary: [repository] MD5 artifact check not done on artifact after non-canoncial download
Product: [Eclipse Project] Equinox Reporter: Nigel Westbury <nigelipse>
Component: p2Assignee: Pascal Rapicault <pascal>
Status: CLOSED WONTFIX QA Contact:
Severity: normal    
Priority: P3 CC: irbull, mn, pascal.rapicault, pascal, tjwatson
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Windows XP   
See Also: https://bugs.eclipse.org/bugs/show_bug.cgi?id=423715
Whiteboard: stalebug
Attachments:
Description Flags
fix to check artifact MD5 none

Description Nigel Westbury CLA 2011-08-08 17:31:12 EDT 
Build Identifier: 3.7

When SimpleArtifactRepository mirrors an artifact using a non-canonical descriptor, the download MD5 is checked but the artifact MD5 is not checked.

This caused a problem because I had patched a jar file in the bundle pool on my desktop.  That was very bad, I know, and I understand that 'all bets are off' on what p2 will do after I do that.  Clearly 'all bets are off' on whether anyone will get the patched or the un-patched jar but what happened to me was that the patched file was use as the base in a jbdiff.  That resulted in an unusable jar for the next version.  The artifact repository had both the download MD5 and the artifact MD5 specified and there does not seem to be much point in having an artifact MD5 property if it is not checked.  The code to do so is trivial (patch attached).  When I added this extra processing step I saw that the artifact MD5 check then failed and the pack 200 was then successfully downloaded.  (If you don't have the patch in bug ? then the canonical will be downloaded).

Doing the artifact MD5 check is necessary if p2 is to be the extremely robust provisioning system that it is designed to be.  Without it any error in the byte content, however it was caused, can cause cascading corrupted versions of the artifact.


Reproducible: Always
Comment 1 Nigel Westbury CLA 2011-08-08 17:35:47 EDT 
Created attachment 201108 [details]
fix to check artifact MD5
Comment 2 Nigel Westbury CLA 2011-08-08 17:37:17 EDT 
The bug I intended to refer to in my original post above is bug 289351.
Comment 3 Pascal Rapicault CLA 2011-08-11 19:40:20 EDT 
Is there a particular reason why you think the MD5 check should not always be on?
Comment 4 Nigel Westbury CLA 2011-08-12 06:47:45 EDT 
I had included a property in my patch to make the artifact MD5 check optional only because there was such a property for the download MD5 check.   I don't know why the download MD5 check shouldn't always be done either except I suppose it can be turned off if a bad repository has incorrect MD5 values, plus if we do artifact checks then doing the download check too is not so necessary.

I have just noticed bug 312802 where turning off the download MD5 check was suggested as a workaround (comment 1) and also comment 5 in the same bug suggesting artifact MD5 checks should be done but that a mis-match should only be a warning.  My vote would certainly be to always do the artifact MD5 check (i.e. romove the property added in my patch) and to make a mis-match always an error because maintaining the integrity of repositories is important.
Comment 5 Thomas Watson CLA 2012-04-30 11:30:13 EDT 
Did this ever get released?  It was marked M2, but still open.
Comment 6 Ian Bull CLA 2012-06-11 14:49:41 EDT 
(In reply to comment #5)
> Did this ever get released?  It was marked M2, but still open.

I don't think so Tom. 

I'm marking this as Kepler because there is a contribution. However, while checking the MD5 after we process (unzip) the non-canonical artifact seems like a great idea, it will likely lead to more cases of bad MD5 dues to Java 7 vs Java 6 jar processing (bug 361628).
Comment 7 Pascal Rapicault CLA 2014-05-05 10:21:22 EDT 
Will not be addressed in Luna.
Comment 8 Eclipse Genie CLA 2019-10-31 07:21:18 EDT 
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.