FELIX-2184 The definitions of PID and configuration property name syntax are actually only recommendations not prescriptions. Thus PIDs may technically be any string and should be accepted from both the party setting up new configuration as well as from persistence. Adding tests to verify this.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk/configadmin@995005 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/src/main/java/org/apache/felix/cm/file/ConfigurationHandler.java b/src/main/java/org/apache/felix/cm/file/ConfigurationHandler.java
index e5c91be..bb4b012 100644
--- a/src/main/java/org/apache/felix/cm/file/ConfigurationHandler.java
+++ b/src/main/java/org/apache/felix/cm/file/ConfigurationHandler.java
@@ -1,4 +1,4 @@
-/*
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -46,7 +46,7 @@
* form a <code>java.io.InputStream</code> and writing to a
* <code>java.io.OutputStream</code> on behalf of the
* {@link FilePersistenceManager} class.
- *
+ *
* <pre>
* cfg = prop "=" value .
* prop = symbolic-name . // 1.4.2 of OSGi Core Specification
@@ -148,6 +148,7 @@
NAME_CHARS.set( '_' );
NAME_CHARS.set( '-' );
NAME_CHARS.set( '.' );
+ NAME_CHARS.set( '\\' );
TOKEN_CHARS = new BitSet();
TOKEN_CHARS.set( TOKEN_EQ );
@@ -186,7 +187,7 @@
* <p>
* This method writes at the current location in the stream and does not
* close the outputstream.
- *
+ *
* @param out
* The <code>OutputStream</code> to write the configurtion data
* to.
@@ -220,7 +221,7 @@
* <p>
* This method reads from the current location in the stream upto the end of
* the stream but does not close the stream at the end.
- *
+ *
* @param ins
* The <code>InputStream</code> from which to read the
* configuration data.
@@ -289,7 +290,7 @@
* value = type ( "[" values "]" | "(" values ")" | simple ) . values =
* value { "," value } . simple = "{" stringsimple "}" . type = // 1-char
* type code . stringsimple = // quoted string representation of the value .
- *
+ *
* @param pr
* @return
* @throws IOException
@@ -320,7 +321,9 @@
return readCollection( type, pr );
case TOKEN_VAL_OPEN:
- return readSimple( type, pr );
+ Object value = readSimple( type, pr );
+ ensureNext( pr, TOKEN_VAL_CLOS );
+ return value;
default:
return null;
@@ -345,6 +348,8 @@
return null;
}
+ ensureNext( pr, TOKEN_VAL_CLOS );
+
list.add( value );
int c = read( pr );
@@ -387,6 +392,8 @@
return null;
}
+ ensureNext( pr, TOKEN_VAL_CLOS );
+
collection.add( value );
int c = read( pr );
@@ -463,6 +470,16 @@
}
+ private void ensureNext( PushbackReader pr, int expected ) throws IOException
+ {
+ int next = read( pr );
+ if ( next != expected )
+ {
+ readFailure( next, expected );
+ }
+ }
+
+
private boolean checkNext( PushbackReader pr, int expected ) throws IOException
{
int next = read( pr );
@@ -527,11 +544,13 @@
// eof
case -1: // fall through
- // separator token
+ // separator token
+ case TOKEN_EQ:
case TOKEN_VAL_CLOS:
+ pr.unread( c );
return buf.toString();
- // no escaping
+ // no escaping
default:
buf.append( ( char ) c );
}
@@ -550,10 +569,11 @@
}
// check whether there is a name
- if ( NAME_CHARS.get( c ) )
+ if ( NAME_CHARS.get( c ) || !TOKEN_CHARS.get( c ) )
{
// read the property name
- tokenValue = readName( pr, ( char ) c );
+ pr.unread( c );
+ tokenValue = readQuoted( pr );
return ( token = TOKEN_NAME );
}
@@ -579,28 +599,6 @@
}
- private String readName( PushbackReader pr, char firstChar ) throws IOException
- {
- StringBuffer buf = new StringBuffer();
- buf.append( firstChar );
-
- int c = read( pr );
- while ( c >= 0 && NAME_CHARS.get( c ) )
- {
- buf.append( ( char ) c );
- c = read( pr );
- }
- pr.unread( c );
-
- if ( buf.charAt( 0 ) == '.' || buf.charAt( buf.length() - 1 ) == '.' )
- {
- throw new IOException( "Name (" + buf + ") must not start or end with a dot" );
- }
-
- return buf.toString();
- }
-
-
private int read( PushbackReader pr ) throws IOException
{
int c = pr.read();
@@ -763,6 +761,8 @@
{
case '\\':
case TOKEN_VAL_CLOS:
+ case ' ':
+ case TOKEN_EQ:
out.write( '\\' );
out.write( c );
break;
diff --git a/src/main/java/org/apache/felix/cm/impl/CaseInsensitiveDictionary.java b/src/main/java/org/apache/felix/cm/impl/CaseInsensitiveDictionary.java
index 0e38864..7c118ca 100644
--- a/src/main/java/org/apache/felix/cm/impl/CaseInsensitiveDictionary.java
+++ b/src/main/java/org/apache/felix/cm/impl/CaseInsensitiveDictionary.java
@@ -267,32 +267,6 @@
{
throw new IllegalArgumentException( "Key [" + key + "] must not be an empty string" );
}
-
- // check for trailing dot
- if ( key.endsWith( "." ) )
- {
- throw new IllegalArgumentException( "Key [" + key + "] must not end with a dot" );
- }
-
- // check for legal characters and non-consecutive dots
- int lastDot = Integer.MIN_VALUE;
- for ( int i = 0; i < key.length(); i++ )
- {
- char c = key.charAt( i );
- if ( c == '.' )
- {
- if ( lastDot == i - 1 )
- {
- throw new IllegalArgumentException( "Key [" + key + "] must not have consecutive dots" );
- }
- lastDot = i;
- }
- else if ( ( c < '0' || c > '9' ) && ( c < 'a' || c > 'z' ) && ( c < 'A' || c > 'Z' ) && c != '_'
- && c != '-' )
- {
- throw new IllegalArgumentException( "Key [" + key + "] contains illegal character" );
- }
- }
}
diff --git a/src/test/java/org/apache/felix/cm/file/FilePersistenceManagerTest.java b/src/test/java/org/apache/felix/cm/file/FilePersistenceManagerTest.java
index 93d3776..f4ef903 100644
--- a/src/test/java/org/apache/felix/cm/file/FilePersistenceManagerTest.java
+++ b/src/test/java/org/apache/felix/cm/file/FilePersistenceManagerTest.java
@@ -1,4 +1,4 @@
-/*
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -97,7 +97,7 @@
public void testQuoting() throws IOException
{
- check( "QuotingSeparators", "\\()[]{}.,=" );
+ check( "QuotingSeparators", "\\()[]{}.,=\"\"''" );
check( "QuotingWellKnown", "BSP:\b, TAB:\t, LF:\n, FF:\f, CR:\r" );
check( "QuotingControl", new String( new char[]
{ 5, 10, 32, 64 } ) );
@@ -122,8 +122,8 @@
check( "IntegerVector", new Vector( Arrays.asList( new Integer[]
{ new Integer( 0 ), new Integer( 1 ), new Integer( 2 ) } ) ) );
}
-
-
+
+
public void testList() throws IOException
{
check( "StringList", Arrays.asList( new String[]
@@ -152,6 +152,16 @@
}
+ // test configuration keys not conforming to the recommended specification
+ // for configuration keys in OSGi CM 1.3, 104.4.2, Configuration Properties
+ public void testNonSpecKeys() throws IOException {
+ check( "with\ttab", "the value" );
+ check( "with blank", "the value" );
+ check( "\\()[]{}.,=\"\"''", "quoted key" );
+ check( "\"with quotes\"", "key with quotes" );
+ check( "=leading equals", "leading equals" );
+ }
+
private void check( String name, Object value ) throws IOException
{
Dictionary props = new Hashtable();
@@ -165,7 +175,7 @@
{
fpm.store( pid, props );
- assertTrue( new File( file, pid + ".config" ).exists() );
+ assertTrue( new File( file, FilePersistenceManager.encodePid( pid ) + ".config" ).exists() );
Dictionary loaded = fpm.load( pid );
assertNotNull( loaded );
diff --git a/src/test/java/org/apache/felix/cm/impl/CaseInsensitiveDictionaryTest.java b/src/test/java/org/apache/felix/cm/impl/CaseInsensitiveDictionaryTest.java
index b83bdb7..d25a830 100644
--- a/src/test/java/org/apache/felix/cm/impl/CaseInsensitiveDictionaryTest.java
+++ b/src/test/java/org/apache/felix/cm/impl/CaseInsensitiveDictionaryTest.java
@@ -210,11 +210,14 @@
public void testKeyDots()
{
- testFailingKey( "." );
+ // FELIX-2184 these keys are all valid (but not recommended)
+ CaseInsensitiveDictionary.checkKey( "." );
+ CaseInsensitiveDictionary.checkKey( "a.b.c." );
+ CaseInsensitiveDictionary.checkKey( ".a.b.c." );
+ CaseInsensitiveDictionary.checkKey( "a..b" );
+
+ // valid key as of OSGi Compendium R4.2 (CM 1.3)
CaseInsensitiveDictionary.checkKey( ".a.b.c" );
- testFailingKey( "a.b.c." );
- testFailingKey( ".a.b.c." );
- testFailingKey( "a..b" );
}
@@ -222,11 +225,13 @@
{
testFailingKey( null );
testFailingKey( "" );
- testFailingKey( " " );
- testFailingKey( "§" );
- testFailingKey( "${yikes}" );
- testFailingKey( "a key with spaces" );
- testFailingKey( "fail:key" );
+
+ // FELIX-2184 these keys are all valid (but not recommended)
+ CaseInsensitiveDictionary.checkKey( " " );
+ CaseInsensitiveDictionary.checkKey( "§" );
+ CaseInsensitiveDictionary.checkKey( "${yikes}" );
+ CaseInsensitiveDictionary.checkKey( "a key with spaces" );
+ CaseInsensitiveDictionary.checkKey( "fail:key" );
}
diff --git a/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java b/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java
index 6e4abb2..ccb8f6f 100644
--- a/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java
+++ b/src/test/java/org/apache/felix/cm/integration/ConfigurationBaseTest.java
@@ -73,6 +73,41 @@
@Test
+ public void test_basic_configuration_strange_pid() throws BundleException, IOException
+ {
+ // 1. create config with pid and locationA
+ // 2. update config with properties
+ final String pid = "pid with blanks and stuff %\"'";
+ theConfig.put( pid, pid );
+ final Configuration config = configure( pid, null, true );
+ theConfig.remove( pid );
+
+ // 3. register ManagedService ms1 with pid from said locationA
+ bundle = installBundle( pid, ManagedServiceTestActivator.class );
+ bundle.start();
+ delay();
+
+ // ==> configuration supplied to the service ms1
+ final ManagedServiceTestActivator tester = ManagedServiceTestActivator.INSTANCE;
+ TestCase.assertNotNull( tester.props );
+ TestCase.assertEquals( pid, tester.props.get( Constants.SERVICE_PID ) );
+ TestCase.assertNull( tester.props.get( ConfigurationAdmin.SERVICE_FACTORYPID ) );
+ TestCase.assertNull( tester.props.get( ConfigurationAdmin.SERVICE_BUNDLELOCATION ) );
+ TestCase.assertEquals( PROP_NAME, tester.props.get( PROP_NAME ) );
+ TestCase.assertEquals( pid, tester.props.get( pid ) );
+ TestCase.assertEquals( 1, tester.numManagedServiceUpdatedCalls );
+
+ // delete
+ config.delete();
+ delay();
+
+ // ==> update with null
+ TestCase.assertNull( tester.props );
+ TestCase.assertEquals( 2, tester.numManagedServiceUpdatedCalls );
+ }
+
+
+ @Test
public void test_basic_configuration_start_then_configure() throws BundleException, IOException
{
final String pid = "test_basic_configuration_start_then_configure";
