diff --git a/src/main/java/org/apache/xmlbeans/impl/schema/SchemaLocalAttributeImpl.java b/src/main/java/org/apache/xmlbeans/impl/schema/SchemaLocalAttributeImpl.java index c46b6577c..545d27748 100644 --- a/src/main/java/org/apache/xmlbeans/impl/schema/SchemaLocalAttributeImpl.java +++ b/src/main/java/org/apache/xmlbeans/impl/schema/SchemaLocalAttributeImpl.java @@ -124,9 +124,9 @@ public XmlAnySimpleType getDefaultValue() { if (_parseObject != null) { + NamespaceContext.push(new NamespaceContext(_parseObject)); try { - NamespaceContext.push(new NamespaceContext(_parseObject)); return getType().newValue(_defaultText); } finally diff --git a/src/main/java/org/apache/xmlbeans/impl/schema/SchemaParticleImpl.java b/src/main/java/org/apache/xmlbeans/impl/schema/SchemaParticleImpl.java index 155536dd6..cb87389cd 100644 --- a/src/main/java/org/apache/xmlbeans/impl/schema/SchemaParticleImpl.java +++ b/src/main/java/org/apache/xmlbeans/impl/schema/SchemaParticleImpl.java @@ -268,8 +268,8 @@ public XmlAnySimpleType getDefaultValue() { } if (_defaultText != null && XmlAnySimpleType.type.isAssignableFrom(getType())) { if (_parseObject != null && XmlQName.type.isAssignableFrom(getType())) { + NamespaceContext.push(new NamespaceContext(_parseObject)); try { - NamespaceContext.push(new NamespaceContext(_parseObject)); return getType().newValue(_defaultText); } finally { NamespaceContext.pop(); diff --git a/src/main/java/org/apache/xmlbeans/impl/values/NamespaceContext.java b/src/main/java/org/apache/xmlbeans/impl/values/NamespaceContext.java index 19a3ced78..cf8a37c07 100644 --- a/src/main/java/org/apache/xmlbeans/impl/values/NamespaceContext.java +++ b/src/main/java/org/apache/xmlbeans/impl/values/NamespaceContext.java @@ -82,8 +82,8 @@ final void push(NamespaceContext next) } final void pop() { - current = stack.get(stack.size() - 1); - stack.remove(stack.size() - 1); + // single remove() so a failure cannot leave current and stack out of step + current = stack.remove(stack.size() - 1); } } @@ -112,16 +112,33 @@ public static void push(NamespaceContext next) public static void pop() { - NamespaceContextStack nsContextStack = getNamespaceContextStack(); + NamespaceContextStack nsContextStack = tl_namespaceContextStack.get(); + if (nsContextStack == null || nsContextStack.stack.isEmpty()) + { + // unbalanced pop - don't throw (it would mask the original exception), just clean up + tl_namespaceContextStack.remove(); + return; + } + nsContextStack.pop(); if (nsContextStack.stack.isEmpty()) - tl_namespaceContextStack.set(null); + tl_namespaceContextStack.remove(); } public static PrefixResolver getCurrent() { - return getNamespaceContextStack().current; + // don't create a stack here: nothing would ever remove it from the thread-local + NamespaceContextStack nsContextStack = tl_namespaceContextStack.get(); + return nsContextStack == null ? null : nsContextStack.current; + } + + /** + * @return true if the current thread has namespace context state (for tests) + */ + static boolean hasThreadLocalState() + { + return tl_namespaceContextStack.get() != null; } public String getNamespaceForPrefix(String prefix) diff --git a/src/test/java/org/apache/xmlbeans/impl/values/NamespaceContextTest.java b/src/test/java/org/apache/xmlbeans/impl/values/NamespaceContextTest.java new file mode 100644 index 000000000..9391544cc --- /dev/null +++ b/src/test/java/org/apache/xmlbeans/impl/values/NamespaceContextTest.java @@ -0,0 +1,72 @@ +/* Copyright 2004 The Apache Software Foundation + * + * Licensed 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.xmlbeans.impl.values; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import java.util.Collections; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +// XMLBEANS-676 +public class NamespaceContextTest { + + @AfterEach + void cleanUp() { + NamespaceContext.clearThreadLocals(); + } + + @Test + void getCurrentDoesNotCreateThreadLocalState() { + assertNull(NamespaceContext.getCurrent()); + assertFalse(NamespaceContext.hasThreadLocalState()); + } + + @Test + void nestedPushPopClearsThreadLocalState() { + NamespaceContext outer = new NamespaceContext(Collections.singletonMap("a", "urn:a")); + NamespaceContext inner = new NamespaceContext(Collections.singletonMap("b", "urn:b")); + + NamespaceContext.push(outer); + NamespaceContext.push(inner); + assertSame(inner, NamespaceContext.getCurrent()); + assertEquals("urn:b", NamespaceContext.getCurrent().getNamespaceForPrefix("b")); + NamespaceContext.pop(); + assertSame(outer, NamespaceContext.getCurrent()); + assertTrue(NamespaceContext.hasThreadLocalState()); + NamespaceContext.pop(); + + assertNull(NamespaceContext.getCurrent()); + assertFalse(NamespaceContext.hasThreadLocalState()); + } + + @Test + void unbalancedPopDoesNotThrowOrLeak() { + assertDoesNotThrow(NamespaceContext::pop); + assertFalse(NamespaceContext.hasThreadLocalState()); + + NamespaceContext.push(new NamespaceContext(Collections.emptyMap())); + NamespaceContext.pop(); + assertDoesNotThrow(NamespaceContext::pop); + assertFalse(NamespaceContext.hasThreadLocalState()); + } +}