From 5308c807267c74a94fdf4c5904b6dc59da5c2f01 Mon Sep 17 00:00:00 2001
From: David Bosschaert <davidb@apache.org>
Date: Tue, 7 Sep 2021 08:37:30 +0100
Subject: [PATCH] Additional parameter checking

This fixes a number of TCK failures
---
 .../feature/impl/ArtifactBuilderImpl.java     | 32 +++++++++++++++----
 .../felix/feature/impl/BundleBuilderImpl.java | 20 +++++++++++-
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/features/src/main/java/org/apache/felix/feature/impl/ArtifactBuilderImpl.java b/features/src/main/java/org/apache/felix/feature/impl/ArtifactBuilderImpl.java
index 683d914ee8..f1c8c3fc39 100644
--- a/features/src/main/java/org/apache/felix/feature/impl/ArtifactBuilderImpl.java
+++ b/features/src/main/java/org/apache/felix/feature/impl/ArtifactBuilderImpl.java
@@ -39,34 +39,52 @@ public FeatureArtifactBuilder addMetadata(String key, Object value) {
     	if (key == null)
     		throw new IllegalArgumentException("Metadata key cannot be null");
 
-    	if (value == null)
-    		throw new IllegalArgumentException("Metadata value cannot be null");
-    	
+    	if (key.length() == 0)
+    		throw new IllegalArgumentException("Key must not be empty");
+
     	if ("id".equalsIgnoreCase(key))
     		throw new IllegalArgumentException("Key cannot be 'id'");
+    	
+    	checkMetadataValue(value);
     	    	
         this.metadata.put(key, value);
         return this;
     }
 
-    @Override
+	@Override
     public FeatureArtifactBuilder addMetadata(Map<String,Object> md) {
     	if (md.keySet().contains(null))
     		throw new IllegalArgumentException("Metadata key cannot be null");
     	
-    	if (md.values().contains(null))
-    		throw new IllegalArgumentException("Metadata value cannot be null");
-    	
+    	if (md.keySet().contains(""))
+    		throw new IllegalArgumentException("Key must not be empty");
+
     	if (md.keySet().stream()
     		.map(String::toLowerCase)
     		.anyMatch(s -> "id".equals(s))) {
     		throw new IllegalArgumentException("Key cannot be 'id'");    		
     	}
+    	
+    	md.values().stream()
+			.forEach(this::checkMetadataValue);
 
     	this.metadata.putAll(md);
         return this;
     }
 
+    private void checkMetadataValue(Object value) {
+    	if (value instanceof String) 
+    		return;
+    	
+    	if (value instanceof Boolean)
+    		return;
+    	
+    	if (value instanceof Number)
+    		return;
+    	
+    	throw new IllegalArgumentException("Illegal metadata value: " + value);
+	}
+
     @Override
     public FeatureArtifact build() {
         return new ArtifactImpl(id, metadata);
diff --git a/features/src/main/java/org/apache/felix/feature/impl/BundleBuilderImpl.java b/features/src/main/java/org/apache/felix/feature/impl/BundleBuilderImpl.java
index 7232dfe5db..275ccb7460 100644
--- a/features/src/main/java/org/apache/felix/feature/impl/BundleBuilderImpl.java
+++ b/features/src/main/java/org/apache/felix/feature/impl/BundleBuilderImpl.java
@@ -45,7 +45,9 @@ public FeatureBundleBuilder addMetadata(String key, Object value) {
     	if ("id".equalsIgnoreCase(key))
     		throw new IllegalArgumentException("Key cannot be 'id'");
     	
-        this.metadata.put(key, value);
+    	checkMetadataValue(value);
+
+    	this.metadata.put(key, value);
         return this;
     }
 
@@ -63,10 +65,26 @@ public FeatureBundleBuilder addMetadata(Map<String,Object> md) {
     		throw new IllegalArgumentException("Key cannot be 'id'");    		
     	}
     		    	
+    	md.values().stream()
+			.forEach(this::checkMetadataValue);
+
         this.metadata.putAll(md);
         return this;
     }
 
+    private void checkMetadataValue(Object value) {
+    	if (value instanceof String) 
+    		return;
+    	
+    	if (value instanceof Boolean)
+    		return;
+    	
+    	if (value instanceof Number)
+    		return;
+    	
+    	throw new IllegalArgumentException("Illegal metadata value: " + value);
+	}
+
     @Override
     public FeatureBundle build() {
         return new BundleImpl(id, metadata);