[MRM-1302] add some prevention for concurrent modification of a list

Merged from: r917398

git-svn-id: https://svn.apache.org/repos/asf/archiva/branches/archiva-1.3.x@917399 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/archiva-modules/archiva-web/archiva-webdav/src/main/java/org/apache/maven/archiva/webdav/ArchivaDavResourceFactory.java b/archiva-modules/archiva-web/archiva-webdav/src/main/java/org/apache/maven/archiva/webdav/ArchivaDavResourceFactory.java
index 2684c8ef2c61e6d7748ba3c761c5914663a1ef48..44260be7501869751c708378c45ba4033ba3664b 100644 (file)
--- a/archiva-modules/archiva-web/archiva-webdav/src/main/java/org/apache/maven/archiva/webdav/ArchivaDavResourceFactory.java
+++ b/archiva-modules/archiva-web/archiva-webdav/src/main/java/org/apache/maven/archiva/webdav/ArchivaDavResourceFactory.java
@@ -88,6 +88,14 @@ import org.codehaus.redback.integration.filter.authentication.HttpAuthenticator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import javax.servlet.http.HttpServletResponse;
+
 /**
  * @plexus.component role="org.apache.maven.archiva.webdav.ArchivaDavResourceFactory"
  */
@@ -206,10 +214,13 @@ public class ArchivaDavResourceFactory
                 return getResource( request, repoGroupConfig.getRepositories(), archivaLocator );
             }
             else
-            {                
-                resource =
-                    processRepositoryGroup( request, archivaLocator, repoGroupConfig.getRepositories(),
-                                            activePrincipal, resourcesInAbsolutePath );                
+            {
+                // make a copy to avoid potential concurrent modifications (eg. by configuration)
+                // TODO: ultimately, locking might be more efficient than copying in this fashion since updates are
+                //  infrequent
+                ArrayList<String> repositories = new ArrayList<String>( repoGroupConfig.getRepositories() );
+                resource = processRepositoryGroup( request, archivaLocator, repositories, activePrincipal,
+                                                   resourcesInAbsolutePath );
             }
         }
         else
@@ -222,8 +233,8 @@ public class ArchivaDavResourceFactory
             }
             catch ( RepositoryNotFoundException e )
             {
-                throw new DavException( HttpServletResponse.SC_NOT_FOUND, "Invalid repository: " +
-                    archivaLocator.getRepositoryId() );
+                throw new DavException( HttpServletResponse.SC_NOT_FOUND,
+                                        "Invalid repository: " + archivaLocator.getRepositoryId() );
             }
             catch ( RepositoryException e )
             {
@@ -231,19 +242,20 @@ public class ArchivaDavResourceFactory
             }
 
             log.debug( "Managed repository '" + managedRepository.getId() + "' accessed by '" + activePrincipal + "'" );
-           
-            resource = processRepository( request, archivaLocator, activePrincipal, managedRepository );           
+
+            resource = processRepository( request, archivaLocator, activePrincipal, managedRepository );
 
             String logicalResource = RepositoryPathUtil.getLogicalResource( locator.getResourcePath() );
-            resourcesInAbsolutePath.add( new File( managedRepository.getRepoRoot(), logicalResource ).getAbsolutePath() );
+            resourcesInAbsolutePath.add( new File( managedRepository.getRepoRoot(),
+                                                   logicalResource ).getAbsolutePath() );
         }
 
         String requestedResource = request.getRequestURI();
 
         // MRM-872 : merge all available metadata
         // merge metadata only when requested via the repo group
-        if ( ( repositoryRequest.isMetadata( requestedResource ) || repositoryRequest.isMetadataSupportFile( requestedResource ) ) &&
-            repoGroupConfig != null )
+        if ( ( repositoryRequest.isMetadata( requestedResource ) || repositoryRequest.isMetadataSupportFile(
+            requestedResource ) ) && repoGroupConfig != null )
         {
             // this should only be at the project level not version level!
             if ( isProjectReference( requestedResource ) )
@@ -252,19 +264,19 @@ public class ArchivaDavResourceFactory
                 artifactId = StringUtils.substringAfterLast( artifactId, "/" );
 
                 ArchivaDavResource res = (ArchivaDavResource) resource;
-                String filePath =
-                    StringUtils.substringBeforeLast( res.getLocalResource().getAbsolutePath().replace( '\\', '/' ), "/" );
+                String filePath = StringUtils.substringBeforeLast( res.getLocalResource().getAbsolutePath().replace(
+                    '\\', '/' ), "/" );
                 filePath = filePath + "/maven-metadata-" + repoGroupConfig.getId() + ".xml";
 
                 // for MRM-872 handle checksums of the merged metadata files
                 if ( repositoryRequest.isSupportFile( requestedResource ) )
                 {
-                    File metadataChecksum =
-                        new File( filePath + "." + StringUtils.substringAfterLast( requestedResource, "." ) );
+                    File metadataChecksum = new File( filePath + "." + StringUtils.substringAfterLast(
+                        requestedResource, "." ) );
                     if ( metadataChecksum.exists() )
                     {
-                        LogicalResource logicalResource =
-                            new LogicalResource( RepositoryPathUtil.getLogicalResource( locator.getResourcePath() ) );
+                        LogicalResource logicalResource = new LogicalResource( RepositoryPathUtil.getLogicalResource(
+                            locator.getResourcePath() ) );
 
                         resource =
                             new ArchivaDavResource( metadataChecksum.getAbsolutePath(), logicalResource.getPath(),
@@ -298,8 +310,8 @@ public class ArchivaDavResourceFactory
                         {
                             File resourceFile = writeMergedMetadataToFile( mergedMetadata, filePath );
 
-                            LogicalResource logicalResource =
-                                new LogicalResource( RepositoryPathUtil.getLogicalResource( locator.getResourcePath() ) );
+                            LogicalResource logicalResource = new LogicalResource(
+                                RepositoryPathUtil.getLogicalResource( locator.getResourcePath() ) );
 
                             resource =
                                 new ArchivaDavResource( resourceFile.getAbsolutePath(), logicalResource.getPath(),
@@ -348,7 +360,7 @@ public class ArchivaDavResourceFactory
 
         for ( String repositoryId : repositories )
         {
-            ManagedRepositoryContent managedRepository = null;
+            ManagedRepositoryContent managedRepository;
             try
             {
                 managedRepository = repositoryFactory.getManagedRepositoryContent( repositoryId );
@@ -364,8 +376,8 @@ public class ArchivaDavResourceFactory
 
             try
             {
-                DavResource updatedResource =
-                    processRepository( request, archivaLocator, activePrincipal, managedRepository );
+                DavResource updatedResource = processRepository( request, archivaLocator, activePrincipal,
+                                                                 managedRepository );
                 if ( resource == null )
                 {
                     resource = updatedResource;
@@ -376,7 +388,8 @@ public class ArchivaDavResourceFactory
                 {
                     logicalResource = logicalResource.substring( 1 );
                 }
-                resourcesInAbsolutePath.add( new File( managedRepository.getRepoRoot(), logicalResource ).getAbsolutePath() );
+                resourcesInAbsolutePath.add( new File( managedRepository.getRepoRoot(),
+                                                       logicalResource ).getAbsolutePath() );
             }
             catch ( DavException e )
             {
@@ -448,8 +461,8 @@ public class ArchivaDavResourceFactory
                         {
                             // Perform an adjustment of the resource to the managed
                             // repository expected path.
-                            String localResourcePath =
-                                repositoryRequest.toNativePath( logicalResource.getPath(), managedRepository );
+                            String localResourcePath = repositoryRequest.toNativePath( logicalResource.getPath(),
+                                                                                       managedRepository );
                             resourceFile = new File( managedRepository.getRepoRoot(), localResourcePath );
                             resource =
                                 new ArchivaDavResource( resourceFile.getAbsolutePath(), logicalResource.getPath(),
@@ -467,9 +480,8 @@ public class ArchivaDavResourceFactory
 
                         if ( fromProxy )
                         {
-                            String event =
-                                ( previouslyExisted ? AuditEvent.MODIFY_FILE : AuditEvent.CREATE_FILE ) +
-                                    PROXIED_SUFFIX;
+                            String event = ( previouslyExisted ? AuditEvent.MODIFY_FILE : AuditEvent.CREATE_FILE ) +
+                                PROXIED_SUFFIX;
 
                             log.debug( "Proxied artifact '" + resourceFile.getName() + "' in repository '" +
                                 managedRepository.getId() + "' (current user '" + activePrincipal + "')" );
@@ -489,7 +501,7 @@ public class ArchivaDavResourceFactory
             if ( request.getMethod().equals( HTTP_PUT_METHOD ) )
             {
                 String resourcePath = logicalResource.getPath();
-                
+
                 // check if target repo is enabled for releases
                 // we suppose that release-artifacts can be deployed only to repos enabled for releases
                 if ( managedRepository.getRepository().isReleases() && !repositoryRequest.isMetadata( resourcePath ) &&
@@ -499,13 +511,15 @@ public class ArchivaDavResourceFactory
                     try
                     {
                         artifact = managedRepository.toArtifactReference( resourcePath );
-                        
+
                         if ( !VersionUtil.isSnapshot( artifact.getVersion() ) )
                         {
                             // check if artifact already exists and if artifact re-deployment to the repository is allowed
-                            if ( managedRepository.hasContent( artifact ) && managedRepository.getRepository().isBlockRedeployments() )
+                            if ( managedRepository.hasContent( artifact ) &&
+                                managedRepository.getRepository().isBlockRedeployments() )
                             {
-                                log.warn( "Overwriting released artifacts in repository '" + managedRepository.getId() + "' is not allowed." );
+                                log.warn( "Overwriting released artifacts in repository '" + managedRepository.getId() +
+                                    "' is not allowed." );
                                 throw new DavException( HttpServletResponse.SC_CONFLICT,
                                                         "Overwriting released artifacts is not allowed." );
                             }
@@ -532,8 +546,9 @@ public class ArchivaDavResourceFactory
                     destDir.mkdirs();
                     String relPath = PathUtil.getRelative( rootDirectory.getAbsolutePath(), destDir );
 
-                    log.debug( "Creating destination directory '" + destDir.getName() + "' (current user '" +
-                        activePrincipal + "')" );
+                    log.debug(
+                        "Creating destination directory '" + destDir.getName() + "' (current user '" + activePrincipal +
+                            "')" );
 
                     triggerAuditEvent( request.getRemoteAddr(), managedRepository.getId(), relPath,
                                        AuditEvent.CREATE_DIR, activePrincipal );
@@ -555,8 +570,8 @@ public class ArchivaDavResourceFactory
         }
         catch ( RepositoryNotFoundException e )
         {
-            throw new DavException( HttpServletResponse.SC_NOT_FOUND, "Invalid repository: " +
-                archivaLocator.getRepositoryId() );
+            throw new DavException( HttpServletResponse.SC_NOT_FOUND,
+                                    "Invalid repository: " + archivaLocator.getRepositoryId() );
         }
         catch ( RepositoryException e )
         {
@@ -622,7 +637,8 @@ public class ArchivaDavResourceFactory
         catch ( ProxyDownloadException e )
         {
             log.error( e.getMessage(), e );
-            throw new DavException( HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Unable to fetch artifact resource." );
+            throw new DavException( HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
+                                    "Unable to fetch artifact resource." );
         }
         return false;
     }
@@ -715,6 +731,7 @@ public class ArchivaDavResourceFactory
     }
 
     // TODO: remove?
+
     private void triggerAuditEvent( String remoteIP, String repositoryId, String resource, String action,
                                     String principal )
     {
@@ -811,9 +828,10 @@ public class ArchivaDavResourceFactory
             AuthenticationResult result = httpAuth.getAuthenticationResult( request, null );
             SecuritySession securitySession = httpAuth.getSecuritySession( request.getSession( true ) );
 
-            return servletAuth.isAuthenticated( request, result ) &&
-                servletAuth.isAuthorized( request, securitySession, repositoryId,
-                                          WebdavMethodUtil.getMethodPermission( request.getMethod() ) );
+            return servletAuth.isAuthenticated( request, result ) && servletAuth.isAuthorized( request, securitySession,
+                                                                                               repositoryId,
+                                                                                               WebdavMethodUtil.getMethodPermission(
+                                                                                                   request.getMethod() ) );
         }
         catch ( AuthenticationException e )
         {
@@ -821,8 +839,7 @@ public class ArchivaDavResourceFactory
             String guest = UserManager.GUEST_USERNAME;
             try
             {
-                if ( servletAuth.isAuthorized(
-                                               guest,
+                if ( servletAuth.isAuthorized( guest,
                                                ( (ArchivaDavResourceLocator) request.getRequestLocator() ).getRepositoryId(),
                                                WebdavMethodUtil.getMethodPermission( request.getMethod() ) ) )
                 {
@@ -915,8 +932,9 @@ public class ArchivaDavResourceFactory
                         catch ( DavException e )
                         {
                             // TODO: review exception handling
-                            log.debug( "Skipping repository '" + managedRepository + "' for user '" + activePrincipal +
-                                "': " + e.getMessage() );
+                            log.debug(
+                                "Skipping repository '" + managedRepository + "' for user '" + activePrincipal + "': " +
+                                    e.getMessage() );
                         }
                     }
                     else
@@ -925,7 +943,8 @@ public class ArchivaDavResourceFactory
                         try
                         {
                             if ( servletAuth.isAuthorized( activePrincipal, repository,
-                                                           WebdavMethodUtil.getMethodPermission( request.getMethod() ) ) )
+                                                           WebdavMethodUtil.getMethodPermission(
+                                                               request.getMethod() ) ) )
                             {
                                 mergedRepositoryContents.add( resourceFile );
                                 log.debug( "Repository '" + repository + "' accessed by '" + activePrincipal + "'" );
@@ -934,8 +953,9 @@ public class ArchivaDavResourceFactory
                         catch ( UnauthorizedException e )
                         {
                             // TODO: review exception handling
-                            log.debug( "Skipping repository '" + managedRepository + "' for user '" + activePrincipal +
-                                "': " + e.getMessage() );
+                            log.debug(
+                                "Skipping repository '" + managedRepository + "' for user '" + activePrincipal + "': " +
+                                    e.getMessage() );
                         }
                     }
                 }
@@ -946,9 +966,9 @@ public class ArchivaDavResourceFactory
             throw new UnauthorizedDavException( locator.getRepositoryId(), "User not authorized." );
         }
 
-        ArchivaVirtualDavResource resource =
-            new ArchivaVirtualDavResource( mergedRepositoryContents, logicalResource.getPath(), mimeTypes, locator,
-                                           this );
+        ArchivaVirtualDavResource resource = new ArchivaVirtualDavResource( mergedRepositoryContents,
+                                                                            logicalResource.getPath(), mimeTypes,
+                                                                            locator, this );
 
         // compatibility with MRM-440 to ensure browsing the repository group works ok
         if ( resource.isCollection() && !request.getRequestURI().endsWith( "/" ) )
@@ -967,7 +987,7 @@ public class ArchivaDavResourceFactory
 
     /**
      * Check if the current user is authorized to access any of the repos
-     * 
+     *
      * @param request
      * @param repositories
      * @param activePrincipal
@@ -1002,8 +1022,8 @@ public class ArchivaDavResourceFactory
             {
                 try
                 {
-                    if ( servletAuth.isAuthorized( activePrincipal, repository,
-                                                   WebdavMethodUtil.getMethodPermission( request.getMethod() ) ) )
+                    if ( servletAuth.isAuthorized( activePrincipal, repository, WebdavMethodUtil.getMethodPermission(
+                        request.getMethod() ) ) )
                     {
                         allow = true;
                         break;