Skip to content

FELIX-6861 Propagate IOExceptions of stream handlers invoked reflectively #575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
paulrutter merged 2 commits into from
Oct 15, 2026
Merged

FELIX-6861 Propagate IOExceptions of stream handlers invoked reflectively #575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04c65f3
FELIX-6861 Propagate IOExceptions of stream handlers invoked reflecti…
sharanggupta Oct 15, 2026
2232120
FELIX-6861 Propagate IOExceptions of stream handler services used thr…
sharanggupta Oct 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 44 additions & 17 deletions framework/src/main/java/org/apache/felix/framework/URLHandlersStreamHandlerProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ protected URLConnection openConnection(URL url) throws IOException
}
catch (Exception ex)
{
rethrowIOException(ex);
throw new IOException(ex.getMessage());
}
}
Expand All @@ -303,6 +304,7 @@ protected URLConnection openConnection(URL url) throws IOException
}
catch (Exception ex)
{
rethrowIOException(ex);

@paulrutter paulrutter Oct 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URLStreamHandlerService fast path still loses the IOException.

The four reflective sites are fixed, but openConnection(URL) returns early at line 266 before reaching any of them:

if (svc instanceof URLStreamHandlerService)
{
    return ((URLStreamHandlerService) svc).openConnection(url);
}

That looks like a direct call, but svc is frequently a dynamic proxy. When the framework's service object is not an instance of this class loader's URLStreamHandlerService — the "handler from another class loader" case named in the PR description — getStreamHandlerService() wraps it at line 622:

return m_action.createProxy(
        m_action.getClassLoader(URLStreamHandlerService.class),
        new Class[]{URLStreamHandlerService.class},
        new URLHandlersStreamHandlerProxy(service, m_action));

That proxy is instanceof URLStreamHandlerService, so it takes the line 266 branch, which dispatches into invoke() at line 660 — m_action.invokeDirect(...) -> Method.invoke. The handler's IOException becomes an InvocationTargetException, and since ITE is not assignable to IOException, the JDK proxy wraps it again in UndeclaredThrowableException.

I confirmed the wrapping with a standalone reproduction of that exact shape:

thrown: java.lang.reflect.UndeclaredThrowableException
  cause: java.lang.reflect.InvocationTargetException
  caught by 'catch (IOException)'? false

So on this path a FileNotFoundException still escapes callers' catch (IOException) blocks as an unchecked exception — the same FELIX-6861 symptom, just with a different wrapper. The call at line 266 isn't inside a try either, so nothing downgrades it.

This is pre-existing rather than introduced here, so I would not hold the PR for it. But since the description frames the fix as covering handlers "from another class loader", it is worth either handling in invoke() (unwrap the ITE there so the proxy sees a declared IOException) or calling out explicitly as out of scope so it isn't assumed fixed.

@sharanggupta sharanggupta Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, you're right, and the PR description promised exactly that case. Fixed in 2232120: invoke() now catches the InvocationTargetException and throws its target exception, as a direct call would. The proxy then propagates the declared IOException of openConnection instead of wrapping it in an UndeclaredThrowableException. handlerServiceOfAnotherClassLoaderPropagatesIOException builds the proxy the way getStreamHandlerService() does, with a service that is not a URLStreamHandlerService. It fails with UndeclaredThrowableException before this commit.

@paulrutter paulrutter Oct 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified on 22321201ce — the fix works and both new tests are genuine regression tests rather than ones that would have passed anyway.

check result
URLHandlersStreamHandlerProxyTest as it stands 5/5 pass
same tests with only invoke() reverted to 04c65f3d3a handlerServiceOfAnotherClassLoaderPropagatesIOException fails with exactly UndeclaredThrowableException
delete just the rethrowIOException(e) at line 340 only openConnectionWithProxyPropagatesIOExceptionOfHandlerService fails
full framework suite 127 tests, 8 failures — the identical pre-existing deleteDir/BundleCacheTest file-locking set I get on master on Windows, unchanged

The last row is the one I most wanted to see, since invoke() is the invocation handler for every proxied method, not just openConnection. Unwrapping the InvocationTargetException there also changes parseURL and the URLStreamHandlerSetter path, and nothing regressed. Throwing getTargetException() is also consistent for the m_service == null branch, which had the same wrapping.

Thanks for turning it around so quickly.

throw new IllegalStateException("Stream handler unavailable due to: " + ex.getMessage(), ex);
}
}
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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.
Expand Down Expand Up @@ -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);
}
}
152 changes: 152 additions & 0 deletions framework/src/test/java/org/apache/felix/framework/URLHandlersStreamHandlerProxyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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

@paulrutter paulrutter Oct 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two of the four changed sites have no test.

rethrowIOException() was added at four places, but the tests reach only two:

site line covered
OPEN_CONNECTION.invoke(svc, url) 299 yes - openConnectionPropagatesIOExceptionOfHandler
OPEN_CONNECTION_PROXY.invoke(svc, url, proxy) 346 yes - openConnectionWithProxyPropagatesIOExceptionOfHandler
extension-manager branch (http://felix.extensions:9) 281 no
URLStreamHandlerService + Proxy branch 336 no

Both tests construct the proxy with a plain URLStreamHandler as the built-in, so getStreamHandlerService() returns it, svc instanceof URLStreamHandlerService is false, and control always reaches the same two sites.

The practical risk is low today, but a later refactor could delete either uncovered rethrowIOException() call and the suite would stay green while the regression returns on those paths. The URLStreamHandlerService + Proxy site is the cheaper of the two to cover: a stub implementing URLStreamHandlerService and declaring openConnection(URL, Proxy) (the getMethod lookup at line 332 requires the latter) that throws a FileNotFoundException.

@sharanggupta sharanggupta Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added openConnectionWithProxyPropagatesIOExceptionOfHandlerService for the URLStreamHandlerService + Proxy branch in 2232120. It uses a stub extending AbstractURLStreamHandlerService with a public openConnection(URL, Proxy) as the built-in handler. The extension-manager branch is still not covered: it needs a framework whose ExtensionManager instance is reachable through the static m_extensionManager field, which this unit test does not set up. If you know an existing test that starts one, I can add a case there.

@paulrutter paulrutter Oct 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no such test, and it is worth knowing why before anyone spends time on one: that branch is unreachable as written, so there is nothing to cover.

Line 277 looks up a static field on ExtensionManager:

m_action.getDeclaredField(ExtensionManager.class, "m_extensionManager", null)

but ExtensionManager declares no field of that name. javap -p on the compiled class lists only:

static final ...ClassPathExtender m_extenderFramework;
static final ...ClassPathExtender m_extenderBoot;
private volatile ...ExtensionManagerRevision m_systemBundleRevision;
private final java.util.List<...ExtensionTuple> m_extensionTuples;

The name m_extensionManager exists only as a private instance field on Felix (Felix.java:119), and getDeclaredField does not search other classes. So the lookup always throws NoSuchFieldException, which lands in the catch (Exception ex) below and becomes throw new IOException(ex.getMessage()) — a URL of the form http://felix.extensions:9 can never return a connection, regardless of whether a framework is running.

So I would leave the branch out of scope here. Your rethrowIOException(ex) call on it is harmless and correct if the lookup is ever repaired; it just cannot be exercised today. That is a separate pre-existing bug from FELIX-6861, and I have not raised a ticket for it — flagging it here so it is on the record for whoever picks it up.

Both findings from my review are addressed as far as this PR is concerned; no further changes needed from me.

{
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);
}
}
Loading