diff --git a/framework/src/main/java/org/apache/felix/framework/URLHandlersStreamHandlerProxy.java b/framework/src/main/java/org/apache/felix/framework/URLHandlersStreamHandlerProxy.java index d2fa1fe61f..6529aaf0c4 100644 --- a/framework/src/main/java/org/apache/felix/framework/URLHandlersStreamHandlerProxy.java +++ b/framework/src/main/java/org/apache/felix/framework/URLHandlersStreamHandlerProxy.java @@ -292,6 +292,7 @@ protected URLConnection openConnection(URL url) throws IOException } catch (Exception ex) { + rethrowIOException(ex); throw new IOException(ex.getMessage()); } } @@ -303,6 +304,7 @@ protected URLConnection openConnection(URL url) throws IOException } catch (Exception ex) { + rethrowIOException(ex); throw new IllegalStateException("Stream handler unavailable due to: " + ex.getMessage(), ex); } } @@ -335,10 +337,7 @@ protected URLConnection openConnection(URL url, java.net.Proxy proxy) throws IOE } catch (Exception e) { - if (e instanceof IOException) - { - throw (IOException) e; - } + rethrowIOException(e); throw new IOException(e.getMessage(), e); } } @@ -348,14 +347,32 @@ protected URLConnection openConnection(URL url, java.net.Proxy proxy) throws IOE } catch (Exception ex) { - if (ex instanceof IOException) - { - throw (IOException) ex; - } + rethrowIOException(ex); throw new IllegalStateException("Stream handler unavailable due to: " + ex.getMessage(), ex); } } + /** + * Rethrows the given exception if it is an {@link IOException}, or the {@link IOException} + * thrown by a stream handler method invoked through reflection, which is wrapped in an + * {@link InvocationTargetException}, so that callers get the same exception as with a + * direct invocation of the handler. + * + * @param ex the exception caught while invoking a stream handler method + * @throws IOException the exception itself or the cause of the invocation exception + */ + private static void rethrowIOException(Exception ex) throws IOException + { + if (ex instanceof IOException) + { + throw (IOException) ex; + } + if ((ex instanceof InvocationTargetException) && (ex.getCause() instanceof IOException)) + { + throw (IOException) ex.getCause(); + } + } + // We use this thread local to detect whether we have a reentrant entry to the parseURL // method. This can happen do to some difference between gnu/classpath and sun jvms // For more see inside the method. @@ -629,18 +646,28 @@ public Object invoke(Object obj, Method method, Object[] params) throws Throwable { Class[] types = method.getParameterTypes(); - if (m_service == null) + try { - return m_action.invoke(m_action.getMethod(this.getClass(), method.getName(), types), this, params); + if (m_service == null) + { + return m_action.invoke(m_action.getMethod(this.getClass(), method.getName(), types), this, params); + } + if ("parseURL".equals(method.getName())) + { + ClassLoader loader = m_action.getClassLoader(m_service.getClass()); + types[0] = loader.loadClass(URLStreamHandlerSetter.class.getName()); + params[0] = m_action.createProxy(loader, new Class[]{types[0]}, + (URLHandlersStreamHandlerProxy) params[0]); + } + return m_action.invokeDirect(m_action.getDeclaredMethod(m_service.getClass(), + method.getName(), types), m_service, params); } - if ("parseURL".equals(method.getName())) + catch (InvocationTargetException ex) { - ClassLoader loader = m_action.getClassLoader(m_service.getClass()); - types[0] = loader.loadClass(URLStreamHandlerSetter.class.getName()); - params[0] = m_action.createProxy(loader, new Class[]{types[0]}, - (URLHandlersStreamHandlerProxy) params[0]); + // Throw what the invoked method threw, as a direct call would, so that the dynamic + // proxy propagates a declared exception, such as the IOException of openConnection, + // instead of wrapping it in an UndeclaredThrowableException (FELIX-6861). + throw ex.getTargetException(); } - return m_action.invokeDirect(m_action.getDeclaredMethod(m_service.getClass(), - method.getName(), types), m_service, params); } } diff --git a/framework/src/test/java/org/apache/felix/framework/URLHandlersStreamHandlerProxyTest.java b/framework/src/test/java/org/apache/felix/framework/URLHandlersStreamHandlerProxyTest.java new file mode 100644 index 0000000000..b3b7edb3ba --- /dev/null +++ b/framework/src/test/java/org/apache/felix/framework/URLHandlersStreamHandlerProxyTest.java @@ -0,0 +1,152 @@ +/* + * 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.felix.framework; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.net.Proxy; +import java.net.URL; +import java.net.URLConnection; +import java.net.URLStreamHandler; + +import org.apache.felix.framework.util.SecureAction; +import org.junit.jupiter.api.Test; +import org.osgi.service.url.AbstractURLStreamHandlerService; +import org.osgi.service.url.URLStreamHandlerService; + +/** + * An {@link IOException} thrown by a stream handler invoked through reflection is propagated + * unchanged (FELIX-6861). + */ +class URLHandlersStreamHandlerProxyTest +{ + private static final FileNotFoundException FAILURE = new FileNotFoundException("no such resource"); + + private static final URLStreamHandler FAILING_HANDLER = new URLStreamHandler() + { + @Override + protected URLConnection openConnection(URL url) throws IOException + { + throw FAILURE; + } + + @Override + protected URLConnection openConnection(URL url, Proxy proxy) throws IOException + { + throw FAILURE; + } + }; + + private static final URLStreamHandler BROKEN_HANDLER = new URLStreamHandler() + { + @Override + protected URLConnection openConnection(URL url) + { + throw new UnsupportedOperationException("broken"); + } + }; + + /** + * A {@link URLStreamHandlerService} whose {@code openConnection(URL, Proxy)} is public, so that the + * proxy invokes it through reflection. + */ + public static class FailingHandlerService extends AbstractURLStreamHandlerService + { + @Override + public URLConnection openConnection(URL url) throws IOException + { + throw FAILURE; + } + + @Override + public URLConnection openConnection(URL url, Proxy proxy) throws IOException + { + throw FAILURE; + } + } + + /** + * A stream handler service that is not a {@link URLStreamHandlerService} of this class loader, as + * when it comes from another class loader, so that it is used through a dynamic proxy. + */ + public static class FailingForeignHandlerService + { + public URLConnection openConnection(URL url) throws IOException + { + throw FAILURE; + } + } + + @Test + void openConnectionPropagatesIOExceptionOfHandler() throws Exception + { + URLHandlersStreamHandlerProxy proxy = new URLHandlersStreamHandlerProxy( + "felixtest", new SecureAction(), FAILING_HANDLER, null); + URL url = new URL(null, "felixtest://host/path", FAILING_HANDLER); + + assertThatThrownBy(() -> proxy.openConnection(url)).isSameAs(FAILURE); + } + + @Test + void openConnectionWithProxyPropagatesIOExceptionOfHandler() throws Exception + { + URLHandlersStreamHandlerProxy proxy = new URLHandlersStreamHandlerProxy( + "felixtest", new SecureAction(), FAILING_HANDLER, null); + URL url = new URL(null, "felixtest://host/path", FAILING_HANDLER); + + assertThatThrownBy(() -> proxy.openConnection(url, Proxy.NO_PROXY)).isSameAs(FAILURE); + } + + @Test + void openConnectionWithProxyPropagatesIOExceptionOfHandlerService() throws Exception + { + FailingHandlerService service = new FailingHandlerService(); + URLHandlersStreamHandlerProxy proxy = new URLHandlersStreamHandlerProxy( + "felixtest", new SecureAction(), service, null); + URL url = new URL(null, "felixtest://host/path", FAILING_HANDLER); + + assertThatThrownBy(() -> proxy.openConnection(url, Proxy.NO_PROXY)).isSameAs(FAILURE); + } + + @Test + void handlerServiceOfAnotherClassLoaderPropagatesIOException() throws Exception + { + SecureAction action = new SecureAction(); + URLStreamHandlerService service = (URLStreamHandlerService) action.createProxy( + URLStreamHandlerService.class.getClassLoader(), new Class[]{URLStreamHandlerService.class}, + new URLHandlersStreamHandlerProxy(new FailingForeignHandlerService(), action)); + URL url = new URL(null, "felixtest://host/path", FAILING_HANDLER); + + assertThatThrownBy(() -> service.openConnection(url)).isSameAs(FAILURE); + } + + @Test + void openConnectionStillWrapsOtherFailuresOfHandler() throws Exception + { + URLHandlersStreamHandlerProxy proxy = new URLHandlersStreamHandlerProxy( + "felixtest", new SecureAction(), BROKEN_HANDLER, null); + URL url = new URL(null, "felixtest://host/path", BROKEN_HANDLER); + + assertThatThrownBy(() -> proxy.openConnection(url)) + .isInstanceOf(IllegalStateException.class) + .hasRootCauseInstanceOf(UnsupportedOperationException.class); + } +}