AzureAD / microsoft-authentication-library-for-java

Microsoft Authentication Library (MSAL) for Java http://aka.ms/aadv2
MIT License
286 stars 143 forks source link

OSGi Manifest imports private com.sun packages #301

Open paul-mcculloch opened 4 years ago

paul-mcculloch commented 4 years ago

The manifest.mf declares a dependency on com.sun.net.httpserver, presumably because some msal4j class imports this package. I guess this fix is to use a different http server - see https://www.oracle.com/java/technologies/faq-sun-packages.html.

This prevents the bundle starting on Apache Karaf (as this package is not exported from the JRE).

The workaround is edit Karaf's custom.properties file and add the package to org.osgi.framework.system.packages.extra

Avery-Dunn commented 4 years ago

Hello @paul-mcculloch : Thanks for bringing this up. We're looking into how to best solve this on the library side so that it works with Apache Karaf without any workarounds, and will update this thread once we have a fix.

In the meantime, does everything work when using the workaround you mentioned? Any other issues we can help with?

SomkaPe commented 4 years ago

@paul-mcculloch please note that mentioned link explains why "sun" packages should not be used. We use "com.sun.net.httpserver" package, which is part of JDK.

You need to expose it, to make it available in OSGI.

paul-mcculloch commented 4 years ago

@SomkaPe Thanks for looking into this. I was looking at Java 8, where I couldn't find any explicit documentation that rules this in or out of the standard class libraries. I see that as of Java 11 this has been made more explicit.

Looking at https://docs.oracle.com/en/java/javase/11/docs/api/index.html I can see that the package is part of the JDK. However it says "The Java Development Kit (JDK) APIs are specific to the JDK and will not necessarily be available in all implementations of the Java SE Platform".

So I think there is still a reasonable case for removing this dependency from the library, so that it is JRE compatible. Let me know if you'd prefer me to log a new issue with a more accurate title.

rgra commented 3 years ago

I'm running into this problem as well during a Tycho build, and the possible workarounds don't seem feasible to me. https://www.eclipse.org/lists/tycho-user/msg09108.html

If you can consider removing the dependency, it would be great.

MushyMiddle commented 2 years ago

I'm running into this as well. It seems to me the root of the problem is that com.sun.net.httpserver is part of the JRE, so shouldn't have to be imported, and in fact isn't really exported anywhere. Perhaps different OSGi platforms handle package visibility differently, but at least for the old-ish Equinox we're using, the package can't be found. FWIW, I'm using Corretto as my JRE, and have verified that com.sun.net.httpserver is available to standard Java apps (non-OSGi). So, it isn't clear to me why msal4j is trying to import something that doesn't exist.

For now, my workaround is to edit the manifest and remove the imports. The bundle at least starts up OK - it remains to be seen if it works, so worst case, I guess I may have to create a stub bundle to export that package until the maintainers decide what to do here.

P.S. This issue is currently listed as an Enhancement. I'm not sure how it got that designation, but it sure seems like a bug to me...

siddhijain commented 2 years ago

@MushyMiddle Thanks for bringing this up. Since many users are facing issues, we will prioritize getting this fixed.