Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
jmanico merged 3 commits into OWASP:main from casid:patch-1
Jul 20, 2021
Merged

Add automatic module name #45

jmanico merged 3 commits into OWASP:main from casid:patch-1
Jul 20, 2021

Conversation

@casid
Copy link
Contributor

@casid casid commented Feb 7, 2021

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 Writer based escape methods are blazingly fast!

Copy link
Contributor Author

casid commented Feb 7, 2021

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

Copy link
Member

jmanico commented Feb 7, 2021 via email

What is your timeline for needing a new release with these changes?
...
-- Jim Manico @manicode
On Feb 7, 2021, at 12:02 AM, Andreas Hager ***@***.***> wrote:  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 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

Copy link
Contributor Author

casid commented Feb 7, 2021

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.

Copy link
Contributor

kwwall commented Feb 7, 2021

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.

Copy link
Member

jmanico commented Feb 7, 2021 via email

The OWASP Java Encoder works as far back at Java 1.5 I believe. - Jim
On 2/7/21 12:30 PM, Kevin W. Wall wrote: 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. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEBYCI6K4OXVYG2TJAESZTS54IAHANCNFSM4XHHQWSQ>.
-- Jim Manico Manicode Security https://www.manicode.com

Copy link
Contributor

kwwall commented Feb 7, 2021 via email

I know, which is why I brought it up. Would not want this PR to result in packaging the Encoder jar in a way that makes it unusable to versions of Java 8 and earlier. Not saying that would happen, but just asking.
...
-kevin
On Sun, Feb 7, 2021, 5:52 PM Jim Manico ***@***.***> wrote: The OWASP Java Encoder works as far back at Java 1.5 I believe. - Jim On 2/7/21 12:30 PM, Kevin W. Wall wrote: > > 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. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > < #45 (comment)>, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/AAEBYCI6K4OXVYG2TJAESZTS54IAHANCNFSM4XHHQWSQ >. > -- Jim Manico Manicode Security https://www.manicode.com — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#45 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAO6PG3PTVJ4YEO4E5HQA5LS54KSFANCNFSM4XHHQWSQ> .

Copy link
Member

jmanico commented Feb 7, 2021 via email

Ahhhh I understand now. Thank you for the heads up!
On 2/7/21 1:05 PM, Kevin W. Wall wrote: I know, which is why I brought it up. Would not want this PR to result in packaging the Encoder jar in a way that makes it unusable to versions of Java 8 and earlier. Not saying that would happen, but just asking. -kevin On Sun, Feb 7, 2021, 5:52 PM Jim Manico ***@***.***> wrote: > The OWASP Java Encoder works as far back at Java 1.5 I believe. > > - Jim > > On 2/7/21 12:30 PM, Kevin W. Wall wrote: > > > > 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. > > > > — > > You are receiving this because you were mentioned. > > Reply to this email directly, view it on GitHub > > < > #45 (comment)>, > > > or unsubscribe > > < > https://github.com/notifications/unsubscribe-auth/AAEBYCI6K4OXVYG2TJAESZTS54IAHANCNFSM4XHHQWSQ > >. > > > -- > Jim Manico > Manicode Security > https://www.manicode.com > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <#45 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAO6PG3PTVJ4YEO4E5HQA5LS54KSFANCNFSM4XHHQWSQ> > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEBYCKB3AD2LQIY4O3C4HDS54MEPANCNFSM4XHHQWSQ>.
-- Jim Manico Manicode Security https://www.manicode.com

Copy link
Contributor Author

casid commented Feb 8, 2021

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/

Copy link
Contributor

kwwall commented Feb 8, 2021

@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. :-)

casid reacted with laugh emoji

Copy link
Member

jmanico commented Feb 8, 2021 via email

My mother did dress me funny thank you so much for making fun of my childhood wounds there Kevin.... •wink•
...
-- Jim Manico @manicode
On Feb 8, 2021, at 4:23 AM, Kevin W. Wall ***@***.***> wrote:  @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. :-) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.
casid reacted with laugh emoji

Copy link
Contributor

kwwall commented Feb 8, 2021 via email

You are welcome! And you aren't the only one. My mother bought me a leisure suit (hangs head in shame) and made me wear it. :D
...
-kevin
On Mon, Feb 8, 2021, 9:56 AM Jim Manico ***@***.***> wrote: My mother did dress me funny thank you so much for making fun of my childhood wounds there Kevin.... •wink• -- Jim Manico @manicode > On Feb 8, 2021, at 4:23 AM, Kevin W. Wall ***@***.***> wrote: > >  > @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. :-) > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub, or unsubscribe. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAO6PGZGXIUGTKTSXEAF3MTS573QHANCNFSM4XHHQWSQ> .
casid reacted with laugh emoji

@jmanico jmanico merged commit 6320a6d into OWASP:main Jul 20, 2021
@casid casid deleted the patch-1 branch July 22, 2021 19:15
Copy link
Contributor Author

casid commented Jul 22, 2021

@jmanico thanks for the merge! Do you know when the next release of owasp-java-encoder is planned?

Copy link
Member

jmanico commented Jul 22, 2021 via email

I don’t.... But is this pressing for you? I can make it happen.
...
On Jul 22, 2021, at 2:19 PM, Andreas Hager ***@***.***> wrote:  @jmanico thanks for the merge! Do you know when the next release of owasp-java-encoder is planned? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

Copy link
Contributor Author

casid commented Jul 23, 2021

No, it's not really pressing. I'll keep a watch on your repo and upgrade once the next release is out. Thanks! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmanico jmanico jmanico approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /