[jira] Closed: (MAVEN-430) maven checks md5 checksums at wrong point.

["Brett Porter (JIRA)" <ji...@codehaus.org>]

     [ http://jira.codehaus.org/browse/MAVEN-430?page=all ]
     
Brett Porter closed MAVEN-430:
------------------------------

    Fix Version:     (was: 1.1-beta-2)
     Resolution: Duplicate

> maven checks md5 checksums at wrong point.
> ------------------------------------------
>
>          Key: MAVEN-430
>          URL: http://jira.codehaus.org/browse/MAVEN-430
>      Project: maven
>         Type: Bug
>   Components: core
>     Versions: 1.0-beta-10
>     Reporter: Brian Ewins

>
>
> In HttpUtils.java, the checksum is downloaded after the jar has already been moved into the repository. This causes jars to be broken in certain circumstances; for example, recently ibiblio was down and was returning a '200' response code with an html page describing the reason for the downtime to every request. As a result, all SNAPSHOT requests downloaded the html and used this corrupted file instead of the jar. Placing the md5 check earlier would have prevented this.
> There is also a logic problem in the current implementation: if a file fails to download (so that tempfile.exists() is false) the section of code which downloads the md5 checksum to the *real* checksum file succeeds. This could potentially cause the md5 to become mismatched to the download. 
> roughly, the code should look like:
>     public static void getFile( String url,
>                                 File destinationFile,
>                                 boolean ignoreErrors,
>                                 boolean useTimestamp,
>                                 String proxyHost,
>                                 String proxyPort,
>                                 String proxyUserName,
>                                 String proxyPassword,
>                                 boolean useChecksum )
>         throws Exception
>     {
>         File tempFile = new File(destinationFile.getParentFile(), destinationFile.getName() + ".incomplete");
>         
>         //No resume at present.
>         if (tempFile.exists()) 
>         {
>             tempFile.delete();
>         
>             if (tempFile.exists()) {
>                 throw new IOException("Unable to remove " + tempFile.getAbsolutePath());    
>             }
>         }
>         
>         
>         // Get the requested file.
>         getFile( url,
>                  tempFile,
>                  ignoreErrors,
>                  useTimestamp,
>                  proxyHost,
>                  proxyPort,
>                  proxyUserName,
>                  proxyPassword );
>             
>         //If it was downloaded, copy it across to the snapshot
>         if (tempFile.exists()) {
>         // Get the checksum if requested.
>         if ( useChecksum )
>         {
>             File checksumTemp = new File( tempFile + ".md5" );
>             File checksumFile = new File( destinationFile + ".md5" );
>             try
>             {
>                 getFile( url + ".md5",
>                          checksumTemp,
>                          ignoreErrors,
>                          useTimestamp,
>                          proxyHost,
>                          proxyPort,
>                          proxyUserName,
>                          proxyPassword );
>                  if (checksumTemp.exists()) {
>                     // TODO: checksum testing logic goes here.
>                     copyFile(checksumTemp, checksumDest);
>                     copyFile(tempFile, destinationFile);
>                     if (!checksumTemp.delete()) {
>                        throw new IOException("Unable to delete " + checksumTemp.getAbsolutePath());                                   
>                     }
>                  } else {
>                     // TODO: should do something more useful
>                     // with this case - where checksum is 
>                     // unreadable but checksum-checking is on.
>                  }
>             }
>             catch ( Exception e )
>             {
>                 // do nothing we will check later in the process
>                 // for the checksums.
>                 // TODO: fix this. the md5 has been checked.
>             }
>         } else {
>             copyFile(tempFile, destinationFile);
>         }
>                     
>             if (!tempFile.delete()) {
>                 throw new IOException("Unable to delete " + tempFile.getAbsolutePath());                                   
>             }
>         }
>        
>     }
>     // refactored method to do file copying, since logic
>     // is used again for md5s.
>     private void copyFile(File tempFile, File destinationFile) {
>             //This stupidity (not renaming) is caused by Win32 file locking stupidity.     
>             FileOutputStream fos = null;
>             FileInputStream fis = null;
>             
>             try {
>                 fis = new FileInputStream(tempFile);
>                 fos = new FileOutputStream(destinationFile);
>                 transferStream(fis, fos);
>             } finally {
>                 try {
>                     fis.close();
>                 } catch (Exception ex) {
>                 }
>                 try {
>                     fos.close();
>                 } catch (Exception ex) {
>                 }
>             }
>             destinationFile.setLastModified(tempFile.lastModified());
>     }

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://jira.codehaus.org/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org