This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-rewriter.git
The following commit(s) were added to refs/heads/master by this push:
new 26976e1 SLING-12308 : Potential ArrayIndexOutOfBoundsException with
global transformers
26976e1 is described below
commit 26976e1b370ef2734428dc80ba3ae2fec3f0351b
Author: Carsten Ziegeler <[email protected]>
AuthorDate: Tue Apr 30 11:01:18 2024 +0200
SLING-12308 : Potential ArrayIndexOutOfBoundsException with global
transformers
---
.../apache/sling/rewriter/impl/FactoryCache.java | 12 +--
.../rewriter/impl/ProcessorConfigurationImpl.java | 29 ++++---
.../impl/TransformerFactoryServiceTracker.java | 14 ++--
.../sling/rewriter/impl/FactoryCacheTest.java | 95 ++++++++++++++++++++++
4 files changed, 125 insertions(+), 25 deletions(-)
diff --git a/src/main/java/org/apache/sling/rewriter/impl/FactoryCache.java
b/src/main/java/org/apache/sling/rewriter/impl/FactoryCache.java
index 8d7331b..90cd237 100644
--- a/src/main/java/org/apache/sling/rewriter/impl/FactoryCache.java
+++ b/src/main/java/org/apache/sling/rewriter/impl/FactoryCache.java
@@ -201,9 +201,9 @@ public class FactoryCache {
* @param factories The transformer factories
* @return The transformer instances
*/
- private Transformer[][] createTransformers(final TransformerFactory[][]
factories) {
- if ( factories == EMPTY_DOUBLE_ARRAY ) {
- return FactoryCache.EMPTY_DOUBLE_ARRAY;
+ Transformer[][] createTransformers(final TransformerFactory[][] factories)
{
+ if ( factories ==
TransformerFactoryServiceTracker.EMPTY_DOUBLE_FACTORY_ARRAY ) {
+ return EMPTY_DOUBLE_ARRAY;
}
final Transformer[][] transformers = new Transformer[2][];
for(int arrayIndex = 0; arrayIndex < 2; arrayIndex++) {
@@ -212,13 +212,15 @@ public class FactoryCache {
if ( factory == null ) count--;
}
if ( count == 0 ) {
- transformers[arrayIndex] = FactoryCache.EMPTY_ARRAY;
+ transformers[arrayIndex] = EMPTY_ARRAY;
} else {
transformers[arrayIndex] = new Transformer[count];
+ int tIndex = 0;
for(int i=0; i < factories[arrayIndex].length; i++) {
final TransformerFactory factory =
factories[arrayIndex][i];
if ( factory != null ) {
- transformers[arrayIndex][i] =
factory.createTransformer();
+ transformers[arrayIndex][tIndex] =
factory.createTransformer();
+ tIndex++;
}
}
}
diff --git
a/src/main/java/org/apache/sling/rewriter/impl/ProcessorConfigurationImpl.java
b/src/main/java/org/apache/sling/rewriter/impl/ProcessorConfigurationImpl.java
index df5f036..33a790e 100644
---
a/src/main/java/org/apache/sling/rewriter/impl/ProcessorConfigurationImpl.java
+++
b/src/main/java/org/apache/sling/rewriter/impl/ProcessorConfigurationImpl.java
@@ -217,7 +217,7 @@ public class ProcessorConfigurationImpl implements
PipelineConfiguration {
pw.println(this.processErrorResponse);
pw.print("Order : ");
pw.println(this.order);
- if (this.name != null) {
+ if (this.name != null) {
pw.print("Active : ");
pw.println(this.isActive);
pw.print("Valid : ");
@@ -281,10 +281,9 @@ public class ProcessorConfigurationImpl implements
PipelineConfiguration {
}
sb.append("processErrorResponse=");
sb.append(this.processErrorResponse);
- sb.append(", ");
+ sb.append(", order=");
+ sb.append(this.order);
if (this.name != null) {
- sb.append("order=");
- sb.append(this.order);
sb.append(", active=");
sb.append(this.isActive);
sb.append(", valid=");
@@ -407,17 +406,17 @@ public class ProcessorConfigurationImpl implements
PipelineConfiguration {
// now check extenstions
// if no extenstion is configured, we apply to all extenstions
if ( this.extensions != null && this.extensions.length > 0 ) {
- boolean found = false;
- int index = 0;
- while ( !found && index < this.extensions.length ) {
- if (
this.extensions[index].equals(processContext.getRequest().getRequestPathInfo().getExtension())
) {
- found = true;
- }
- index++;
- }
- if ( !found ) {
- return false;
- }
+ boolean found = false;
+ int index = 0;
+ while ( !found && index < this.extensions.length ) {
+ if (
this.extensions[index].equals(processContext.getRequest().getRequestPathInfo().getExtension())
) {
+ found = true;
+ }
+ index++;
+ }
+ if ( !found ) {
+ return false;
+ }
}
// check resource types
if ( this.resourceTypes != null && this.resourceTypes.length > 0 ) {
diff --git
a/src/main/java/org/apache/sling/rewriter/impl/TransformerFactoryServiceTracker.java
b/src/main/java/org/apache/sling/rewriter/impl/TransformerFactoryServiceTracker.java
index 45f0cc7..5e996ef 100644
---
a/src/main/java/org/apache/sling/rewriter/impl/TransformerFactoryServiceTracker.java
+++
b/src/main/java/org/apache/sling/rewriter/impl/TransformerFactoryServiceTracker.java
@@ -125,12 +125,16 @@ final class TransformerFactoryServiceTracker extends
HashingServiceTrackerCustom
int index = 0;
for(final ServiceReference<TransformerFactory> ref :
refs) {
if ( isGlobal(ref) ) {
- if ( index < preCount ) {
- LOGGER.debug("Initializing pre global
TransformerFactory for service ref: {}", ref.getClass());
- globalFactories[0][index] = new
TransformerFactoryEntry(this.getService(ref), ref);
+ LOGGER.debug("Initializing {} global
TransformerFactory for service ref: {}", index < preCount ? "pre" : "post",
ref.getClass());
+ final TransformerFactory factory =
this.getService(ref);
+ if ( factory == null) {
+ LOGGER.debug("TransformerFactory is null
for service ref: {}", ref);
} else {
- LOGGER.debug("Initializing post global
TransformerFactory for service ref: {}", ref.getClass());
- globalFactories[1][index - preCount] = new
TransformerFactoryEntry(this.getService(ref), ref);
+ if ( index < preCount ) {
+ globalFactories[0][index] = new
TransformerFactoryEntry(factory, ref);
+ } else {
+ globalFactories[1][index - preCount] =
new TransformerFactoryEntry(factory, ref);
+ }
}
index++;
}
diff --git a/src/test/java/org/apache/sling/rewriter/impl/FactoryCacheTest.java
b/src/test/java/org/apache/sling/rewriter/impl/FactoryCacheTest.java
new file mode 100644
index 0000000..f9f11a9
--- /dev/null
+++ b/src/test/java/org/apache/sling/rewriter/impl/FactoryCacheTest.java
@@ -0,0 +1,95 @@
+/*
+ * 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 regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package org.apache.sling.rewriter.impl;
+
+import static org.junit.Assert.assertEquals;
+
+import org.apache.sling.rewriter.Transformer;
+import org.apache.sling.rewriter.TransformerFactory;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.InvalidSyntaxException;
+
+public class FactoryCacheTest {
+
+ @Test
+ public void testCreateTransformers() throws InvalidSyntaxException {
+ final BundleContext bc = Mockito.mock(BundleContext.class);
+ final FactoryCache cache = new FactoryCache(bc);
+
+ final TransformerFactory tf1 = Mockito.mock(TransformerFactory.class);
+ final Transformer t1 = Mockito.mock(Transformer.class);
+ Mockito.when(tf1.createTransformer()).thenReturn(t1);
+
+ // empty double array
+ Transformer[][] result =
cache.createTransformers(TransformerFactoryServiceTracker.EMPTY_DOUBLE_FACTORY_ARRAY);
+ assertEquals(2, result.length);
+ assertEquals(0, result[0].length);
+ assertEquals(0, result[1].length);
+
+ // first empty second t1
+ final TransformerFactory[][] factories = new TransformerFactory[][] {
TransformerFactoryServiceTracker.EMPTY_FACTORY_ARRAY, new TransformerFactory[]
{ tf1 } };
+ result = cache.createTransformers(factories);
+ assertEquals(2, result.length);
+ assertEquals(0, result[0].length);
+ assertEquals(1, result[1].length);
+ assertEquals(t1, result[1][0]);
+
+ // first t1 second empty
+ final TransformerFactory[][] factories2 = new TransformerFactory[][] {
new TransformerFactory[] { tf1 },
TransformerFactoryServiceTracker.EMPTY_FACTORY_ARRAY };
+ result = cache.createTransformers(factories2);
+ assertEquals(2, result.length);
+ assertEquals(1, result[0].length);
+ assertEquals(0, result[1].length);
+ assertEquals(t1, result[0][0]);
+
+ // first t1 second t1
+ final TransformerFactory[][] factories3 = new TransformerFactory[][] {
new TransformerFactory[] { tf1 }, new TransformerFactory[] { tf1 } };
+ result = cache.createTransformers(factories3);
+ assertEquals(2, result.length);
+ assertEquals(1, result[0].length);
+ assertEquals(1, result[1].length);
+ assertEquals(t1, result[0][0]);
+ assertEquals(t1, result[1][0]);
+
+ // create second transformer factory
+ final TransformerFactory tf2 = Mockito.mock(TransformerFactory.class);
+ final Transformer t2 = Mockito.mock(Transformer.class);
+ Mockito.when(tf2.createTransformer()).thenReturn(t2);
+
+ // first t1 and t2, second t2 and t1
+ final TransformerFactory[][] factories4 = new TransformerFactory[][] {
new TransformerFactory[] { tf1, tf2 }, new TransformerFactory[] { tf2, tf1 } };
+ result = cache.createTransformers(factories4);
+ assertEquals(2, result.length);
+ assertEquals(2, result[0].length);
+ assertEquals(2, result[1].length);
+ assertEquals(t1, result[0][0]);
+ assertEquals(t2, result[0][1]);
+ assertEquals(t2, result[1][0]);
+ assertEquals(t1, result[1][1]);
+
+ // first t1 and null, second null and t1
+ final TransformerFactory[][] factories5 = new TransformerFactory[][] {
new TransformerFactory[] { tf1, null }, new TransformerFactory[] { null, tf1 }
};
+ result = cache.createTransformers(factories5);
+ assertEquals(2, result.length);
+ assertEquals(1, result[0].length);
+ assertEquals(1, result[1].length);
+ assertEquals(t1, result[0][0]);
+ assertEquals(t1, result[1][0]);
+ }
+}
