-
Couldn't load subscription status.
- Fork 122
Add automatic module name #45
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.
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
Conversation
Sorry for the rushed initial PR. I noticed that there are three modules, not just one. I reverted the initial change and added the automatic module name via apache felix bundle plugin. When building locally I now get the desired output for each module.
Please feel free to adjust the proposed module names as you see fit:
- org.owasp.encoder
- org.owasp.encoder.esapi
- org.owasp.encoder.jsp
Hey @jmanico, thank you for looking into it!
There's no rush on my end. I'm happy that I consolidated my packages and have now an automatic module name set for all jte modules.
Whenever this is merged and part of a owasp encoder release, I'll move forward.
I was under the (possibly mistaken) belief that the Java modules officially introduced in Java 9 and based on JSR 376 would not necessarily be compatible for use by earlier versions of Java. Java modules seems like a great idea but that belief is one reason that ESAPI has been avoiding them. If that would mean that those using Java 8 and earlier can no longer use the Java Encoder, I think that would be a show stopper. I've only read more about the concept but don't know many of the implementation details, which is why I am asking about it. Thanks.
Hey @kwwall thank you for looking into it.
I think older Java versions will work fine. This PR adds an Automatic-Module-Name entry to each MANIFEST.MF file, nothing else.
For the core module:
Automatic-Module-Name: org.owasp.encoder
For the esapi module:
Automatic-Module-Name: org.owasp.encoder.esapi
For the jsp module:
Automatic-Module-Name: org.owasp.encoder.jsp
Java versions before 9 don't know and ignore that entry. Java versions 9 and greater however can use the automatic module name to reference those jars on the module path.
The only harm of this PR I can think of is, that I picked bad names for the modules, since it would be hard to change them later on, without potentially breaking builds who referenced them in their module info. E.g. I'm not sure if you'd like to have org.owasp.encoder or org.owasp.encoder.core for the core module.
There's some further information in these blog post:
http://branchandbound.net/blog/java/2017/12/automatic-module-name/
https://blog.frankel.ch/hard-look-state-java-modularization/
@casid - Thanks for the useful links. I agree, if all this does is result in adding entries for Automatic-Module-Name to MANIFEST.MF in the jar (which those linked posts seem to confirm), that it indeed should cause no problems. My original concern was that it would maybe do something to the .class files contained within the jar so that they would no longer work with versions of Java prior to Java 9. Good to see that is not the case.
As for the "bad names for the modules", that is something that needs to be chosen by the OWASP Java Encoder maintainers. Although, personally, I think org.owasp.encoder.jim_manicos_mother_dresses_him_funny would be a better name than than what you suggested, even if @jmanico doesn't agree. Fortunately, I don't get a vote. :-)
@jmanico thanks for the merge! Do you know when the next release of owasp-java-encoder is planned?
No, it's not really pressing. I'll keep a watch on your repo and upgrade once the next release is out. Thanks! 👍
Hey, I'm using OWASP Java Encoder in my template engine (https://jte.gg).
I'm currently adding Java 9 modules to jte and noticed that OWASP Java Encoder does not yet have an automatic module name. This would allow me to reference the encoder in my module.info.
What do you think?
PS: Thanks for this awesome library, the
Writerbased escape methods are blazingly fast!