From 578d82f2b73f67f7e745b132d0b9195b5c49190e Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 10 Dec 2020 14:14:16 +0100 Subject: [PATCH 1/6] MDEPLOY-279: Better validation for alt deploy repository mojo parameter Do not accept anything but expected format. Fail always if not expected alt repo string passed in, but improve error messages best possible. IF legacy used, provide useful error message giving solution as well. --- .../maven/plugins/deploy/DeployMojo.java | 60 ++++++++++---- .../maven/plugins/deploy/DeployMojoTest.java | 81 ++++++++++++++++++- 2 files changed, 126 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index 5bc471d7..da8dcbcd 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -19,13 +19,6 @@ * under the License. */ -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - import org.apache.maven.artifact.ArtifactUtils; import org.apache.maven.artifact.repository.ArtifactRepository; import org.apache.maven.plugin.MojoExecutionException; @@ -41,6 +34,13 @@ import org.apache.maven.shared.transfer.project.deploy.ProjectDeployer; import org.apache.maven.shared.transfer.project.deploy.ProjectDeployerRequest; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + /** * Deploys an artifact to remote repository. * @@ -51,6 +51,9 @@ public class DeployMojo extends AbstractDeployMojo { + private static final Pattern ALT_INVALID_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+)::(.+)::(.+)::(.+)" ); + + private static final Pattern ALT_LEGACY_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+)::(.+)::(.+)" ); private static final Pattern ALT_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+)::(.+)" ); @@ -248,19 +251,48 @@ else if ( !ArtifactUtils.isSnapshot( project.getVersion() ) && altReleaseDeploym { getLog().info( "Using alternate deployment repository " + altDeploymentRepo ); - Matcher matcher = ALT_REPO_SYNTAX_PATTERN.matcher( altDeploymentRepo ); + Matcher matcher = ALT_INVALID_REPO_SYNTAX_PATTERN.matcher( altDeploymentRepo ); - if ( !matcher.matches() ) + if ( matcher.matches() ) { - throw new MojoFailureException( altDeploymentRepo, "Invalid syntax for repository.", - "Invalid syntax for alternative repository. Use \"id::url\"." ); + throw new MojoFailureException( altDeploymentRepo, + "Invalid syntax for repository.", + "Invalid syntax for alternative repository. Use \"id::url\"." + ); } else { - String id = matcher.group( 1 ).trim(); - String url = matcher.group( 2 ).trim(); + matcher = ALT_LEGACY_REPO_SYNTAX_PATTERN.matcher( altDeploymentRepo ); - repo = createDeploymentArtifactRepository( id, url ); + if ( matcher.matches() ) + { + String id = matcher.group( 1 ).trim(); + String url = matcher.group( 3 ).trim(); + + throw new MojoFailureException( altDeploymentRepo, + "Invalid legacy syntax for repository.", + "Invalid legacy syntax for alternative repository. Use \"" + id + "::" + url + "\" instead." + ); + } + else + { + matcher = ALT_REPO_SYNTAX_PATTERN.matcher( altDeploymentRepo ); + + if ( !matcher.matches() ) + { + throw new MojoFailureException( altDeploymentRepo, + "Invalid syntax for repository.", + "Invalid syntax for alternative repository. Use \"id::url\"." + ); + } + else + { + String id = matcher.group( 1 ).trim(); + String url = matcher.group( 2 ).trim(); + + repo = createDeploymentArtifactRepository( id, url ); + } + } } } diff --git a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java index ee762ef4..a06db0bc 100644 --- a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java @@ -32,6 +32,7 @@ import org.apache.maven.artifact.repository.ArtifactRepository; import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugin.MojoFailureException; import org.apache.maven.plugin.testing.AbstractMojoTestCase; import org.apache.maven.plugin.testing.stubs.MavenProjectStub; import org.apache.maven.plugins.deploy.stubs.ArtifactDeployerStub; @@ -551,7 +552,85 @@ public void _testBasicDeployWithScpAsProtocol() FileUtils.deleteDirectory( sshFile ); } - + + public void testLegacyAltDeploymentRepositoryWithDefaultLayout() + throws Exception + { + DeployMojo mojo = spy( new DeployMojo() ); + + ArtifactRepository repository = mock( ArtifactRepository.class ); + when( mojo.createDeploymentArtifactRepository( "altDeploymentRepository", "http://localhost" + ) ).thenReturn( repository ); + + project.setVersion( "1.0-SNAPSHOT" ); + + ProjectDeployerRequest pdr = + new ProjectDeployerRequest() + .setProject( project ) + .setAltDeploymentRepository( "altDeploymentRepository::default::http://localhost" ); + try + { + mojo.getDeploymentRepository( pdr ); + fail( "Should throw: Invalid legacy syntax for repository." ); + } + catch( MojoFailureException e ) + { + assertEquals( e.getMessage(), "Invalid legacy syntax for repository."); + } + } + + public void testLegacyAltDeploymentRepositoryWithLegacyLayout() + throws Exception + { + DeployMojo mojo = spy( new DeployMojo() ); + + ArtifactRepository repository = mock( ArtifactRepository.class ); + when( mojo.createDeploymentArtifactRepository( "altDeploymentRepository", "http://localhost" + ) ).thenReturn( repository ); + + project.setVersion( "1.0-SNAPSHOT" ); + + ProjectDeployerRequest pdr = + new ProjectDeployerRequest() + .setProject( project ) + .setAltDeploymentRepository( "altDeploymentRepository::legacy::http://localhost" ); + try + { + mojo.getDeploymentRepository( pdr ); + fail( "Should throw: Invalid legacy syntax for repository." ); + } + catch( MojoFailureException e ) + { + assertEquals( e.getMessage(), "Invalid legacy syntax for repository."); + } + } + + public void testInsaneAltDeploymentRepositoryWithLegacyLayout() + throws Exception + { + DeployMojo mojo = spy( new DeployMojo() ); + + ArtifactRepository repository = mock( ArtifactRepository.class ); + when( mojo.createDeploymentArtifactRepository( "altDeploymentRepository", "http://localhost" + ) ).thenReturn( repository ); + + project.setVersion( "1.0-SNAPSHOT" ); + + ProjectDeployerRequest pdr = + new ProjectDeployerRequest() + .setProject( project ) + .setAltDeploymentRepository( "altDeploymentRepository::hey::wow::foo::http://localhost" ); + try + { + mojo.getDeploymentRepository( pdr ); + fail( "Should throw: Invalid syntax for repository." ); + } + catch( MojoFailureException e ) + { + assertEquals( e.getMessage(), "Invalid syntax for repository."); + } + } + public void testAltSnapshotDeploymentRepository() throws Exception { From 27d86ee19f4c64535abd82e7ea4f18c390e935cf Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 15 Dec 2020 18:43:40 +0100 Subject: [PATCH 2/6] Get rid of catch-all :) make them non-greedy --- .../maven/plugins/deploy/DeployMojo.java | 46 +++++++------------ .../maven/plugins/deploy/DeployMojoTest.java | 7 ++- 2 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index da8dcbcd..8aa94d2b 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -51,11 +51,9 @@ public class DeployMojo extends AbstractDeployMojo { - private static final Pattern ALT_INVALID_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+)::(.+)::(.+)::(.+)" ); + private static final Pattern ALT_LEGACY_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+)::(.+)" ); - private static final Pattern ALT_LEGACY_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+)::(.+)::(.+)" ); - - private static final Pattern ALT_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+)::(.+)" ); + private static final Pattern ALT_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+)" ); /** * When building with multiple threads, reaching the last project doesn't have to mean that all projects are ready @@ -251,47 +249,35 @@ else if ( !ArtifactUtils.isSnapshot( project.getVersion() ) && altReleaseDeploym { getLog().info( "Using alternate deployment repository " + altDeploymentRepo ); - Matcher matcher = ALT_INVALID_REPO_SYNTAX_PATTERN.matcher( altDeploymentRepo ); + Matcher matcher = ALT_LEGACY_REPO_SYNTAX_PATTERN.matcher( altDeploymentRepo ); if ( matcher.matches() ) { + String id = matcher.group( 1 ).trim(); + String url = matcher.group( 3 ).trim(); + throw new MojoFailureException( altDeploymentRepo, - "Invalid syntax for repository.", - "Invalid syntax for alternative repository. Use \"id::url\"." + "Invalid legacy syntax for repository.", + "Invalid legacy syntax for alternative repository. Use \"" + id + "::" + url + "\" instead." ); } else { - matcher = ALT_LEGACY_REPO_SYNTAX_PATTERN.matcher( altDeploymentRepo ); + matcher = ALT_REPO_SYNTAX_PATTERN.matcher( altDeploymentRepo ); - if ( matcher.matches() ) + if ( !matcher.matches() ) { - String id = matcher.group( 1 ).trim(); - String url = matcher.group( 3 ).trim(); - throw new MojoFailureException( altDeploymentRepo, - "Invalid legacy syntax for repository.", - "Invalid legacy syntax for alternative repository. Use \"" + id + "::" + url + "\" instead." + "Invalid syntax for repository.", + "Invalid syntax for alternative repository. Use \"id::url\"." ); } else { - matcher = ALT_REPO_SYNTAX_PATTERN.matcher( altDeploymentRepo ); - - if ( !matcher.matches() ) - { - throw new MojoFailureException( altDeploymentRepo, - "Invalid syntax for repository.", - "Invalid syntax for alternative repository. Use \"id::url\"." - ); - } - else - { - String id = matcher.group( 1 ).trim(); - String url = matcher.group( 2 ).trim(); - - repo = createDeploymentArtifactRepository( id, url ); - } + String id = matcher.group( 1 ).trim(); + String url = matcher.group( 2 ).trim(); + + repo = createDeploymentArtifactRepository( id, url ); } } } diff --git a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java index a06db0bc..c6ebf617 100644 --- a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java @@ -576,6 +576,7 @@ public void testLegacyAltDeploymentRepositoryWithDefaultLayout() catch( MojoFailureException e ) { assertEquals( e.getMessage(), "Invalid legacy syntax for repository."); + assertEquals( e.getLongMessage(), "Invalid legacy syntax for alternative repository. Use \"altDeploymentRepository::http://localhost\" instead."); } } @@ -602,6 +603,7 @@ public void testLegacyAltDeploymentRepositoryWithLegacyLayout() catch( MojoFailureException e ) { assertEquals( e.getMessage(), "Invalid legacy syntax for repository."); + assertEquals( e.getLongMessage(), "Invalid legacy syntax for alternative repository. Use \"altDeploymentRepository::http://localhost\" instead."); } } @@ -623,11 +625,12 @@ public void testInsaneAltDeploymentRepositoryWithLegacyLayout() try { mojo.getDeploymentRepository( pdr ); - fail( "Should throw: Invalid syntax for repository." ); + fail( "Should throw: Invalid legacy syntax for repository." ); } catch( MojoFailureException e ) { - assertEquals( e.getMessage(), "Invalid syntax for repository."); + assertEquals( e.getMessage(), "Invalid legacy syntax for repository."); + assertEquals( e.getLongMessage(), "Invalid legacy syntax for alternative repository. Use \"altDeploymentRepository::http://localhost\" instead."); } } From 6a24b7c31d23b334325cbb0a5b501cd2935398fc Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 15 Dec 2020 18:46:02 +0100 Subject: [PATCH 3/6] Fix UT method name (was C+P) --- .../java/org/apache/maven/plugins/deploy/DeployMojoTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java index c6ebf617..89387d42 100644 --- a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java @@ -607,7 +607,7 @@ public void testLegacyAltDeploymentRepositoryWithLegacyLayout() } } - public void testInsaneAltDeploymentRepositoryWithLegacyLayout() + public void testInsaneAltDeploymentRepository() throws Exception { DeployMojo mojo = spy( new DeployMojo() ); From c2c63b27ae50437a52de0231d19861b8d18bc00e Mon Sep 17 00:00:00 2001 From: Michael Osipov Date: Sun, 20 Dec 2020 22:38:26 +0100 Subject: [PATCH 4/6] Restore import order --- .../apache/maven/plugins/deploy/DeployMojo.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index 8aa94d2b..8571d0a0 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -19,6 +19,13 @@ * under the License. */ +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import org.apache.maven.artifact.ArtifactUtils; import org.apache.maven.artifact.repository.ArtifactRepository; import org.apache.maven.plugin.MojoExecutionException; @@ -34,13 +41,6 @@ import org.apache.maven.shared.transfer.project.deploy.ProjectDeployer; import org.apache.maven.shared.transfer.project.deploy.ProjectDeployerRequest; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - /** * Deploys an artifact to remote repository. * From f3a87fbaf9f3c3b5776bd245ce66772517cb1da7 Mon Sep 17 00:00:00 2001 From: Michael Osipov Date: Sun, 20 Dec 2020 22:38:58 +0100 Subject: [PATCH 5/6] Be non-greedy for the repo layout --- src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index 8571d0a0..d6f345be 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -51,7 +51,7 @@ public class DeployMojo extends AbstractDeployMojo { - private static final Pattern ALT_LEGACY_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+)::(.+)" ); + private static final Pattern ALT_LEGACY_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+?)::(.+)" ); private static final Pattern ALT_REPO_SYNTAX_PATTERN = Pattern.compile( "(.+?)::(.+)" ); From a1a5c61b31c8b7a2849d8427b9d8cf7a2daa8dce Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 20 Dec 2020 23:38:55 +0100 Subject: [PATCH 6/6] Make error say only default layout is supported. Extend tests as well. --- .../maven/plugins/deploy/DeployMojo.java | 20 ++++-- .../maven/plugins/deploy/DeployMojoTest.java | 61 +++++++++++++++++-- 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java index d6f345be..ab0c72b2 100644 --- a/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java +++ b/src/main/java/org/apache/maven/plugins/deploy/DeployMojo.java @@ -254,12 +254,24 @@ else if ( !ArtifactUtils.isSnapshot( project.getVersion() ) && altReleaseDeploym if ( matcher.matches() ) { String id = matcher.group( 1 ).trim(); + String layout = matcher.group( 2 ).trim(); String url = matcher.group( 3 ).trim(); - throw new MojoFailureException( altDeploymentRepo, - "Invalid legacy syntax for repository.", - "Invalid legacy syntax for alternative repository. Use \"" + id + "::" + url + "\" instead." - ); + if ( "default".equals( layout ) ) + { + throw new MojoFailureException( altDeploymentRepo, + "Invalid legacy syntax for repository.", + "Invalid legacy syntax for alternative repository. Use \"" + id + "::" + url + "\" instead." + ); + } + else + { + throw new MojoFailureException( altDeploymentRepo, + "Invalid legacy syntax and layout for repository.", + "Invalid legacy syntax and layout for alternative repository. Use \"" + + id + "::" + url + "\" instead, and only default layout is supported." + ); + } } else { diff --git a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java index 89387d42..9a02b1c4 100644 --- a/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java +++ b/src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java @@ -598,12 +598,12 @@ public void testLegacyAltDeploymentRepositoryWithLegacyLayout() try { mojo.getDeploymentRepository( pdr ); - fail( "Should throw: Invalid legacy syntax for repository." ); + fail( "Should throw: Invalid legacy syntax and layout for repository." ); } catch( MojoFailureException e ) { - assertEquals( e.getMessage(), "Invalid legacy syntax for repository."); - assertEquals( e.getLongMessage(), "Invalid legacy syntax for alternative repository. Use \"altDeploymentRepository::http://localhost\" instead."); + assertEquals( e.getMessage(), "Invalid legacy syntax and layout for repository."); + assertEquals( e.getLongMessage(), "Invalid legacy syntax and layout for alternative repository. Use \"altDeploymentRepository::http://localhost\" instead, and only default layout is supported."); } } @@ -623,6 +623,33 @@ public void testInsaneAltDeploymentRepository() .setProject( project ) .setAltDeploymentRepository( "altDeploymentRepository::hey::wow::foo::http://localhost" ); try + { + mojo.getDeploymentRepository( pdr ); + fail( "Should throw: Invalid legacy syntax and layout for repository." ); + } + catch( MojoFailureException e ) + { + assertEquals( e.getMessage(), "Invalid legacy syntax and layout for repository."); + assertEquals( e.getLongMessage(), "Invalid legacy syntax and layout for alternative repository. Use \"altDeploymentRepository::wow::foo::http://localhost\" instead, and only default layout is supported."); + } + } + + public void testDefaultScmSvnAltDeploymentRepository() + throws Exception + { + DeployMojo mojo = spy( new DeployMojo() ); + + ArtifactRepository repository = mock( ArtifactRepository.class ); + when( mojo.createDeploymentArtifactRepository( "altDeploymentRepository", "http://localhost" + ) ).thenReturn( repository ); + + project.setVersion( "1.0-SNAPSHOT" ); + + ProjectDeployerRequest pdr = + new ProjectDeployerRequest() + .setProject( project ) + .setAltDeploymentRepository( "altDeploymentRepository::default::scm:svn:http://localhost" ); + try { mojo.getDeploymentRepository( pdr ); fail( "Should throw: Invalid legacy syntax for repository." ); @@ -630,7 +657,33 @@ public void testInsaneAltDeploymentRepository() catch( MojoFailureException e ) { assertEquals( e.getMessage(), "Invalid legacy syntax for repository."); - assertEquals( e.getLongMessage(), "Invalid legacy syntax for alternative repository. Use \"altDeploymentRepository::http://localhost\" instead."); + assertEquals( e.getLongMessage(), "Invalid legacy syntax for alternative repository. Use \"altDeploymentRepository::scm:svn:http://localhost\" instead."); + } + } + public void testLegacyScmSvnAltDeploymentRepository() + throws Exception + { + DeployMojo mojo = spy( new DeployMojo() ); + + ArtifactRepository repository = mock( ArtifactRepository.class ); + when( mojo.createDeploymentArtifactRepository( "altDeploymentRepository", "http://localhost" + ) ).thenReturn( repository ); + + project.setVersion( "1.0-SNAPSHOT" ); + + ProjectDeployerRequest pdr = + new ProjectDeployerRequest() + .setProject( project ) + .setAltDeploymentRepository( "altDeploymentRepository::legacy::scm:svn:http://localhost" ); + try + { + mojo.getDeploymentRepository( pdr ); + fail( "Should throw: Invalid legacy syntax and layout for repository." ); + } + catch( MojoFailureException e ) + { + assertEquals( e.getMessage(), "Invalid legacy syntax and layout for repository."); + assertEquals( e.getLongMessage(), "Invalid legacy syntax and layout for alternative repository. Use \"altDeploymentRepository::scm:svn:http://localhost\" instead, and only default layout is supported."); } }