search

md-5 / SpecialSource

Automatic generator and renamer of jar obfuscation mappings.
Other
202 stars 51 forks source link

Always close the InputStreams returned from Jar.getResource #46

Closed LunNova closed 6 years ago

LunNova commented 6 years ago

Jar.getResource returns an InputStream which should be closed. It isn't always.

Might fix #45, might not. 😖 Fixes #45

LunNova commented 6 years ago

Force pushed to fix a whitespace error.

mezz commented 6 years ago

This, combined with some similar fixes in FG, fixes my issue in #45.

mezz commented 6 years ago

@md-5 could you create an SS build with this change and without the Java 9 changes? It is possible to fix my issue with this change, but it may be hard for me to convince FG to update to using an SS with preliminary Java 9 support.

AbrarSyed commented 6 years ago

looking at the commit.. seems all you did was update to ASM6? I dont think that will be a huge breaking change unless the generated bytecode is significantly different...

mezz commented 6 years ago

Well it did work fine with the mods I tested, but I wouldn't want to break someone somehow. If it's fine then that makes things a lot easier.

md-5 commented 6 years ago

ASM6 is not backwards compatible with ASM5, and in some cases those incompatibilities aren't obvious until runtime (eg the change to RemappingClassAdapter). If ForgeGradle uses ASM outside of via SpecialSource (I'm guessing it does), there is a good chance it will need a change somewhere. The bytecode should be the same however once its actually working.

LunNova commented 6 years ago

Thanks for the review. Hopefully it's okay now.

The change to containsClass in a separate commit as I'm not sure if you actually want it as it changes the behaviour if trying to open the stream would have thrown.