I tried to write a patch for this, and I don't think this is really
possible to do without introducing some sort of state in order to
avoid duplicate messages, I also ran into the issue that a repository
definition might not be used as it might be overridden by another
definition, so issuing warnings when repository objects gets created
isn't really possible.
After looking around a bit and trying different things I came to the
conclusion that the listeners might be a resonable place to hold this
state and issue warnings, that might of course be totally wrong as I'm
not very familiar with the code.
I attached a patch that added a hash of domains that there has been a
issued a warning about, so that maven doesn't warn twice about the
same domain. The hash is not threadsafe, because as far as I
understood the code the listeners doesn't run in multiple threads. It
issues one warning per unique hostname that it downloads from.
//Alex
2016-10-08 12:03 GMT+02:00 Alexander Kjäll <[email protected]>:
> I liked the idea to only issue warnings about repository urls and not
> for every download, that would greatly reduce the amount of duplicated
> information.
>
> I think it might be user friendly to inform when someone has
> configured their project so that it disables the security model of
> maven, but maybe a warning is to strong and it should rather be an
> info level message?
>
> It might not be obvious to everyone that it's actually the security of
> the transport layer that is a key feature, and if that is compromised
> it's game over. For example at my place of work some thought that it
> didn't matter with https since there was checksums of all the
> artifacts.
>
> I agree that not all repositories have https, but now that lets
> encrypt exists it's only a simple configuration tweak to add it, so I
> think the improved security of adding https outweights the
> inconvenience of it.
>
> //Alex
>
> 2016-10-08 11:34 GMT+02:00 Robert Scholte <[email protected]>:
>> It should be possible to run any build without a warning. We cannot assume
>> that every http connection also has a https connection. Maven is only aware
>> of one URL and that's the one to Central. This has already been changed to
>> https. Other URL's are specified in the settings.xml, (direct) pom.xml and
>> dependency-poms. The first two are managed by the end-user, he has set these
>> values so he already should be aware of these values.
>> The dependency poms (and plugin poms) are harder to discover and to control.
>> For all cases having a repository manager is much easier to control
>> connections.
>> If there should be a warning, might be better to write an enforcer-rule for
>> it and apply it on your own projects.
>>
>> Robert
>>
>>
>> On Sat, 08 Oct 2016 00:49:36 +0200, Manfred Moser <[email protected]>
>> wrote:
>>
>>> The aether code is currently absorbed into Maven so you just need to hang
>>> tight until thats done if you want to propose a code change. But its right
>>> here to the same team.
>>>
>>> And regarding the warning ... such a warning would have to be disabled by
>>> default otherwise it would litter the log for many existing builds causing
>>> all sorts of issues. And then I am not sure it makes much sense.
>>>
>>> But say you go with a warning you would not want to warn for each
>>> download but only for the first one to avoid excessive logging. So maybe
>>> just warn for each specific repository URL once.
>>>
>>> Manfred
>>>
>>> Alexander Kjäll wrote on 2016-10-07 15:42:
>>>
>>>> Thats good feedback, I'll investigate the aether code and propose the
>>>> same thing to them.
>>>>
>>>> I agree that some people might want to have their download unsecure,
>>>> that's why I think that a warning is an appropriate level of
>>>> notification regarding this.
>>>>
>>>> //Alex
>>>>
>>>> 2016-10-08 0:16 GMT+02:00 Michael Osipov <[email protected]>:
>>>>>
>>>>> Am 2016-10-07 um 23:31 schrieb Alexander Kjäll:
>>>>>>
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> I would like to propose that maven issues a warning when an artifacts
>>>>>> gets downloaded over http instead of https.
>>>>>>
>>>>>> The current security model kind of relies on that noone MITM's the
>>>>>> download and replaces the artifact and checksums with something
>>>>>> malicious. That becomes impossible to guarantee when run over a
>>>>>> transport layer that lacks security.
>>>>>>
>>>>>> I have attached a very crude patch that implements this behaviour, but
>>>>>> I'm sure it needs to be reworked before it's ready to be merged.
>>>>>
>>>>>
>>>>>
>>>>> Basically, Aether should handle this, as you might plug other protocols
>>>>> to
>>>>> pull from: SFTP, FTPS, DAVS, etc. Additionally, if this happens in a
>>>>> company, maybe people are quite fine with unsecure only.
>>>>>
>>>>> To sum up: we should wait when Aether transforms to Maven Artifact
>>>>> Resolver.
>>>>>
>>>>> Michael
>>>>>
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: [email protected]
>>>>> For additional commands, e-mail: [email protected]
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: [email protected]
>>>> For additional commands, e-mail: [email protected]
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [email protected]
>>> For additional commands, e-mail: [email protected]
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [email protected]
>> For additional commands, e-mail: [email protected]
>>
diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
index 822f696..7ca8b7e 100644
--- a/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
+++ b/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java
@@ -64,6 +64,7 @@
import org.apache.maven.cli.logging.Slf4jConfigurationFactory;
import org.apache.maven.cli.logging.Slf4jLoggerManager;
import org.apache.maven.cli.logging.Slf4jStdoutLogger;
+import org.apache.maven.cli.transfer.AbstractMavenTransferListener;
import org.apache.maven.cli.transfer.ConsoleMavenTransferListener;
import org.apache.maven.cli.transfer.QuietMavenTransferListener;
import org.apache.maven.cli.transfer.Slf4jMavenTransferListener;
@@ -1751,7 +1752,8 @@ public ExitException( int exitCode )
protected TransferListener getConsoleTransferListener( boolean printResourceNames )
{
- return new ConsoleMavenTransferListener( System.out, printResourceNames );
+ return new ConsoleMavenTransferListener( System.out, printResourceNames,
+ plexusLoggerManager.getLoggerForComponent( AbstractMavenTransferListener.class.getName() ) );
}
protected TransferListener getBatchTransferListener()
diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/transfer/AbstractMavenTransferListener.java b/maven-embedder/src/main/java/org/apache/maven/cli/transfer/AbstractMavenTransferListener.java
index e72aa47..1169f9c 100644
--- a/maven-embedder/src/main/java/org/apache/maven/cli/transfer/AbstractMavenTransferListener.java
+++ b/maven-embedder/src/main/java/org/apache/maven/cli/transfer/AbstractMavenTransferListener.java
@@ -19,17 +19,22 @@
* under the License.
*/
-import java.io.PrintStream;
-import java.text.DecimalFormat;
-import java.text.DecimalFormatSymbols;
-import java.util.Locale;
-
import org.apache.commons.lang3.Validate;
+import org.codehaus.plexus.logging.Logger;
import org.eclipse.aether.transfer.AbstractTransferListener;
import org.eclipse.aether.transfer.TransferCancelledException;
import org.eclipse.aether.transfer.TransferEvent;
import org.eclipse.aether.transfer.TransferResource;
+import java.io.PrintStream;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.text.DecimalFormat;
+import java.text.DecimalFormatSymbols;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Set;
+
public abstract class AbstractMavenTransferListener
extends AbstractTransferListener
{
@@ -207,10 +212,27 @@ public String formatProgress( long progressedSize, long size )
}
protected PrintStream out;
+ private Logger logger;
- protected AbstractMavenTransferListener( PrintStream out )
+ private Set<String> warnedDomains = new HashSet<>();
+
+ protected AbstractMavenTransferListener( PrintStream out, Logger logger )
{
this.out = out;
+ this.logger = logger;
+ }
+
+ private String getHost( String url )
+ {
+ try
+ {
+ URI uri = new URI( url );
+ return uri.getHost();
+ }
+ catch ( URISyntaxException e )
+ {
+ return "";
+ }
}
@Override
@@ -219,6 +241,14 @@ public void transferInitiated( TransferEvent event )
String type = event.getRequestType() == TransferEvent.RequestType.PUT ? "Uploading" : "Downloading";
TransferResource resource = event.getResource();
+ String host = getHost( resource.getRepositoryUrl() );
+ if ( resource.getRepositoryUrl() != null && resource.getRepositoryUrl().startsWith( "http://"; )
+ && !warnedDomains.contains( host ) )
+ {
+ warnedDomains.add( host );
+ logger.warn( "downloading over insecure transport layer from host " + host
+ + ", please use https instead of http." );
+ }
out.println( type + ": " + resource.getRepositoryUrl() + resource.getResourceName() );
}
@@ -227,8 +257,8 @@ public void transferCorrupted( TransferEvent event )
throws TransferCancelledException
{
TransferResource resource = event.getResource();
- out.println( "[WARNING] " + event.getException().getMessage() + " for " + resource.getRepositoryUrl()
- + resource.getResourceName() );
+ logger.warn( event.getException().getMessage() + " for " + resource.getRepositoryUrl()
+ + resource.getResourceName() );
}
@Override
diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/transfer/BatchModeMavenTransferListener.java b/maven-embedder/src/main/java/org/apache/maven/cli/transfer/BatchModeMavenTransferListener.java
index 0e20f17..761c409 100644
--- a/maven-embedder/src/main/java/org/apache/maven/cli/transfer/BatchModeMavenTransferListener.java
+++ b/maven-embedder/src/main/java/org/apache/maven/cli/transfer/BatchModeMavenTransferListener.java
@@ -19,14 +19,16 @@
* under the License.
*/
+import org.codehaus.plexus.logging.Logger;
+
import java.io.PrintStream;
public class BatchModeMavenTransferListener
extends AbstractMavenTransferListener
{
- public BatchModeMavenTransferListener( PrintStream out )
+ public BatchModeMavenTransferListener( PrintStream out, Logger logger )
{
- super( out );
+ super( out, logger );
}
}
diff --git a/maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java b/maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java
index a61f2e1..83501fa 100644
--- a/maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java
+++ b/maven-embedder/src/main/java/org/apache/maven/cli/transfer/ConsoleMavenTransferListener.java
@@ -27,6 +27,7 @@
import java.util.Map;
import org.apache.commons.lang3.StringUtils;
+import org.codehaus.plexus.logging.Logger;
import org.eclipse.aether.transfer.TransferCancelledException;
import org.eclipse.aether.transfer.TransferEvent;
import org.eclipse.aether.transfer.TransferResource;
@@ -46,9 +47,9 @@
private boolean printResourceNames;
private int lastLength;
- public ConsoleMavenTransferListener( PrintStream out, boolean printResourceNames )
+ public ConsoleMavenTransferListener( PrintStream out, boolean printResourceNames, Logger logger )
{
- super( out );
+ super( out, logger );
this.printResourceNames = printResourceNames;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
