From ece827a6ec5da8b36865d1ed3e4e6de1e2a7665a Mon Sep 17 00:00:00 2001 From: Benjamin Bentmann Date: Sun, 1 Jan 2012 17:08:50 +0100 Subject: [PATCH] Fixed handling of properties in DefaultRepositorySystemSession - Made get*Properties() provide read-only views - Made copy constructor clone incoming maps to avoid unintended data coupling with input session --- .../util/DefaultRepositorySystemSession.java | 73 ++++++++++--------- .../DefaultRepositorySystemSessionTest.java | 54 ++++++++++++++ 2 files changed, 92 insertions(+), 35 deletions(-) diff --git a/aether-util/src/main/java/org/eclipse/aether/util/DefaultRepositorySystemSession.java b/aether-util/src/main/java/org/eclipse/aether/util/DefaultRepositorySystemSession.java index 9cb7289b..d322a528 100644 --- a/aether-util/src/main/java/org/eclipse/aether/util/DefaultRepositorySystemSession.java +++ b/aether-util/src/main/java/org/eclipse/aether/util/DefaultRepositorySystemSession.java @@ -10,9 +10,8 @@ *******************************************************************************/ package org.eclipse.aether.util; +import java.util.Collections; import java.util.HashMap; -import java.util.Hashtable; -import java.util.LinkedHashMap; import java.util.Map; import org.eclipse.aether.RepositoryCache; @@ -84,10 +83,16 @@ public final class DefaultRepositorySystemSession private Map systemProperties = new HashMap(); + private Map systemPropertiesView = Collections.unmodifiableMap( systemProperties ); + private Map userProperties = new HashMap(); + private Map userPropertiesView = Collections.unmodifiableMap( userProperties ); + private Map configProperties = new HashMap(); + private Map configPropertiesView = Collections.unmodifiableMap( configProperties ); + private MirrorSelector mirrorSelector = NullMirrorSelector.INSTANCE; private ProxySelector proxySelector = NullProxySelector.INSTANCE; @@ -383,7 +388,20 @@ public final class DefaultRepositorySystemSession return this; } - private Map toSafeMap( Map table, Class valueType ) + private Map copy( Map map ) + { + if ( map == null || map.isEmpty() ) + { + map = new HashMap(); + } + else + { + map = new HashMap( map ); + } + return map; + } + + private Map copySafe( Map table, Class valueType ) { Map map; if ( table == null || table.isEmpty() ) @@ -392,7 +410,7 @@ public final class DefaultRepositorySystemSession } else { - map = new LinkedHashMap(); + map = new HashMap(); for ( Map.Entry entry : table.entrySet() ) { Object key = entry.getKey(); @@ -411,7 +429,7 @@ public final class DefaultRepositorySystemSession public Map getSystemProperties() { - return systemProperties; + return systemPropertiesView; } /** @@ -423,14 +441,8 @@ public final class DefaultRepositorySystemSession */ public DefaultRepositorySystemSession setSystemProperties( Map systemProperties ) { - if ( systemProperties == null ) - { - this.systemProperties = new HashMap(); - } - else - { - this.systemProperties = systemProperties; - } + this.systemProperties = copy( systemProperties ); + systemPropertiesView = Collections.unmodifiableMap( this.systemProperties ); return this; } @@ -441,9 +453,10 @@ public final class DefaultRepositorySystemSession * @param systemProperties The system properties, may be {@code null} or empty if none. * @return This session for chaining, never {@code null}. */ - public DefaultRepositorySystemSession setSystemProps( Hashtable systemProperties ) + public DefaultRepositorySystemSession setSystemProps( Map systemProperties ) { - this.systemProperties = toSafeMap( systemProperties, String.class ); + this.systemProperties = copySafe( systemProperties, String.class ); + systemPropertiesView = Collections.unmodifiableMap( this.systemProperties ); return this; } @@ -469,7 +482,7 @@ public final class DefaultRepositorySystemSession public Map getUserProperties() { - return userProperties; + return userPropertiesView; } /** @@ -482,14 +495,8 @@ public final class DefaultRepositorySystemSession */ public DefaultRepositorySystemSession setUserProperties( Map userProperties ) { - if ( userProperties == null ) - { - this.userProperties = new HashMap(); - } - else - { - this.userProperties = userProperties; - } + this.userProperties = copy( userProperties ); + userPropertiesView = Collections.unmodifiableMap( this.userProperties ); return this; } @@ -503,7 +510,8 @@ public final class DefaultRepositorySystemSession */ public DefaultRepositorySystemSession setUserProps( Map userProperties ) { - this.userProperties = toSafeMap( userProperties, String.class ); + this.userProperties = copySafe( userProperties, String.class ); + userPropertiesView = Collections.unmodifiableMap( this.userProperties ); return this; } @@ -529,7 +537,7 @@ public final class DefaultRepositorySystemSession public Map getConfigProperties() { - return configProperties; + return configPropertiesView; } /** @@ -541,14 +549,8 @@ public final class DefaultRepositorySystemSession */ public DefaultRepositorySystemSession setConfigProperties( Map configProperties ) { - if ( configProperties == null ) - { - this.configProperties = new HashMap(); - } - else - { - this.configProperties = configProperties; - } + this.configProperties = copy( configProperties ); + configPropertiesView = Collections.unmodifiableMap( this.configProperties ); return this; } @@ -561,7 +563,8 @@ public final class DefaultRepositorySystemSession */ public DefaultRepositorySystemSession setConfigProps( Map configProperties ) { - this.configProperties = toSafeMap( configProperties, Object.class ); + this.configProperties = copySafe( configProperties, Object.class ); + configPropertiesView = Collections.unmodifiableMap( this.configProperties ); return this; } diff --git a/aether-util/src/test/java/org/eclipse/aether/util/DefaultRepositorySystemSessionTest.java b/aether-util/src/test/java/org/eclipse/aether/util/DefaultRepositorySystemSessionTest.java index 4d245e94..92d61f54 100644 --- a/aether-util/src/test/java/org/eclipse/aether/util/DefaultRepositorySystemSessionTest.java +++ b/aether-util/src/test/java/org/eclipse/aether/util/DefaultRepositorySystemSessionTest.java @@ -47,4 +47,58 @@ public class DefaultRepositorySystemSessionTest assertSame( repo.getAuthentication(), session.getAuthenticationSelector().getAuthentication( repo ) ); } + @Test + public void testCopyConstructorCopiesPropertiesDeep() + { + DefaultRepositorySystemSession session1 = new DefaultRepositorySystemSession(); + session1.setUserProps( System.getProperties() ); + session1.setSystemProps( System.getProperties() ); + session1.setConfigProps( System.getProperties() ); + + DefaultRepositorySystemSession session2 = new DefaultRepositorySystemSession( session1 ); + session2.setUserProperty( "key", "test" ); + session2.setSystemProperty( "key", "test" ); + session2.setConfigProperty( "key", "test" ); + + assertEquals( null, session1.getUserProperties().get( "key" ) ); + assertEquals( null, session1.getSystemProperties().get( "key" ) ); + assertEquals( null, session1.getConfigProperties().get( "key" ) ); + } + + @Test + public void testReadOnlyProperties() + { + DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(); + + try + { + session.getUserProperties().put( "key", "test" ); + fail( "user properties are modifiable" ); + } + catch ( UnsupportedOperationException e ) + { + // expected + } + + try + { + session.getSystemProperties().put( "key", "test" ); + fail( "system properties are modifiable" ); + } + catch ( UnsupportedOperationException e ) + { + // expected + } + + try + { + session.getConfigProperties().put( "key", "test" ); + fail( "config properties are modifiable" ); + } + catch ( UnsupportedOperationException e ) + { + // expected + } + } + } -- GitLab