OWASP / owasp-java-encoder

The OWASP Java Encoder is a Java 1.5+ simple-to-use drop-in high-performance encoder class with no dependencies and little baggage. This project will help Java web developers defend against Cross Site Scripting!
https://owasp.org/www-project-java-encoder/
BSD 3-Clause "New" or "Revised" License
493 stars 111 forks source link

Changes to get owasp-java-encoder to work with ESAPI 2.2.0.0 and later #37

Closed kwwall closed 4 years ago

kwwall commented 4 years ago

Close #31

Note that you will need to do an official release for this. I did not update your owasp-java-encoder version because I wasn't sure if you wanted to call it 1.2.3 or 1.3.0 or what.

IMPORTANT NOTE Your minimal JDK is 1.5. As of ESAPI 2.2, or minimal supported JDK is 1.7. Please make your users aware of that should they choose to use this never version of org.owasp.encoder:encoder-esapi

jmanico commented 4 years ago

Hey Kevin,

Does this bring in a dependency on ESAPI to the Encoder?

Why are we adding ESAPI code to the encoder as opposed to moving this ESAPI shell code the the ESAPI project?

-- Jim Manico @Manicode

On Jul 29, 2020, at 11:37 PM, Kevin W. Wall notifications@github.com wrote:

 Close #31

Note that you will need to do an official release for this. I did not update your owasp-java-encoder version because I wasn't sure if you wanted to call it 1.2.3 or 1.3.0 or what.

IMPORTANT NOTE Your minimal JDK is 1.5. As of ESAPI 2.2, or minimal supported JDK is 1.7. Please make your users aware of that should they choose to use this never version of org.owasp.encoder:encoder-esapi

You can view, comment on, or merge this pull request online at:

https://github.com/OWASP/owasp-java-encoder/pull/37

Commit Summary

Bump min ESAPI dependency from 2.0 to 2.2. Bring ESAPIEncoder in compliance with ESAPI 2.2.0.0 and later Encoder interface which added new methods. Minimal properties to get JUnit tests working for ESAPIEncoderTest. File Changes

M esapi/pom.xml (2) M esapi/src/main/java/org/owasp/encoder/esapi/ESAPIEncoder.java (14) M esapi/src/test/resources/.esapi/ESAPI.properties (39) Patch Links:

https://github.com/OWASP/owasp-java-encoder/pull/37.patch https://github.com/OWASP/owasp-java-encoder/pull/37.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

kwwall commented 4 years ago

I just was following the pattern that you guys started and it already had a dependency on ESAPI in that shim.

Personally, I thought it would always have made more sense for your project to just put this out there as example code. If you don't want the dependency on ESAPI's DefaultEncoder, you could always just throw a RuntimeException of "Method not implemented" (although technically that would be breaking the interface contract).

Were it not for a few additional dependencies, it would make way more sense to do this from the ESAPI side. Maybe when we get to ESAPI 3 and I can abandon backward compatibility of the 2.0 interfaces. But today ESAPI already has too many dependencies.

But given that anyone who would be using this is already going to be using ESAPI, I don't think the dependency on ESAPI is that much of a big deal. (And if you're not using ESAPI already why would you ever use this instead of using the Java Encoder project directly?)

-kevin

Blog: http://off-the-wall-security.blogspot.com/ | Twitter: @KevinWWall NSA: All your crypto bit are belong to us.

On Thu, Jul 30, 2020, 00:00 Jim Manico notifications@github.com wrote:

Hey Kevin,

Does this bring in a dependency on ESAPI to the Encoder?

Why are we adding ESAPI code to the encoder as opposed to moving this ESAPI shell code the the ESAPI project?

-- Jim Manico @Manicode

On Jul 29, 2020, at 11:37 PM, Kevin W. Wall notifications@github.com wrote:

 Close #31

Note that you will need to do an official release for this. I did not update your owasp-java-encoder version because I wasn't sure if you wanted to call it 1.2.3 or 1.3.0 or what.

IMPORTANT NOTE Your minimal JDK is 1.5. As of ESAPI 2.2, or minimal supported JDK is 1.7. Please make your users aware of that should they choose to use this never version of org.owasp.encoder:encoder-esapi

You can view, comment on, or merge this pull request online at:

https://github.com/OWASP/owasp-java-encoder/pull/37

Commit Summary

Bump min ESAPI dependency from 2.0 to 2.2. Bring ESAPIEncoder in compliance with ESAPI 2.2.0.0 and later Encoder interface which added new methods. Minimal properties to get JUnit tests working for ESAPIEncoderTest. File Changes

M esapi/pom.xml (2) M esapi/src/main/java/org/owasp/encoder/esapi/ESAPIEncoder.java (14) M esapi/src/test/resources/.esapi/ESAPI.properties (39) Patch Links:

https://github.com/OWASP/owasp-java-encoder/pull/37.patch https://github.com/OWASP/owasp-java-encoder/pull/37.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/OWASP/owasp-java-encoder/pull/37#issuecomment-666085771, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAO6PG2G5TNKZK5TK3HG5ODR6DV6BANCNFSM4PMWKHKA .

jmanico commented 4 years ago

I'm teaching today but will look into this tonight!

On 7/30/20 1:16 AM, Kevin W. Wall wrote:

I just was following the pattern that you guys started and it already had a dependency on ESAPI in that shim.

Personally, I thought it would always have made more sense for your project to just put this out there as example code. If you don't want the dependency on ESAPI's DefaultEncoder, you could always just throw a RuntimeException of "Method not implemented" (although technically that would be breaking the interface contract).

Were it not for a few additional dependencies, it would make way more sense to do this from the ESAPI side. Maybe when we get to ESAPI 3 and I can abandon backward compatibility of the 2.0 interfaces. But today ESAPI already has too many dependencies.

But given that anyone who would be using this is already going to be using ESAPI, I don't think the dependency on ESAPI is that much of a big deal. (And if you're not using ESAPI already why would you ever use this instead of using the Java Encoder project directly?)

-kevin

Blog: http://off-the-wall-security.blogspot.com/ | Twitter: @KevinWWall NSA: All your crypto bit are belong to us.

On Thu, Jul 30, 2020, 00:00 Jim Manico notifications@github.com wrote:

Hey Kevin,

Does this bring in a dependency on ESAPI to the Encoder?

Why are we adding ESAPI code to the encoder as opposed to moving this ESAPI shell code the the ESAPI project?

-- Jim Manico @Manicode

On Jul 29, 2020, at 11:37 PM, Kevin W. Wall notifications@github.com wrote:

 Close #31

Note that you will need to do an official release for this. I did not update your owasp-java-encoder version because I wasn't sure if you wanted to call it 1.2.3 or 1.3.0 or what.

IMPORTANT NOTE Your minimal JDK is 1.5. As of ESAPI 2.2, or minimal supported JDK is 1.7. Please make your users aware of that should they choose to use this never version of org.owasp.encoder:encoder-esapi

You can view, comment on, or merge this pull request online at:

https://github.com/OWASP/owasp-java-encoder/pull/37

Commit Summary

Bump min ESAPI dependency from 2.0 to 2.2. Bring ESAPIEncoder in compliance with ESAPI 2.2.0.0 and later Encoder interface which added new methods. Minimal properties to get JUnit tests working for ESAPIEncoderTest. File Changes

M esapi/pom.xml (2) M esapi/src/main/java/org/owasp/encoder/esapi/ESAPIEncoder.java (14) M esapi/src/test/resources/.esapi/ESAPI.properties (39) Patch Links:

https://github.com/OWASP/owasp-java-encoder/pull/37.patch https://github.com/OWASP/owasp-java-encoder/pull/37.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub

https://github.com/OWASP/owasp-java-encoder/pull/37#issuecomment-666085771, or unsubscribe

https://github.com/notifications/unsubscribe-auth/AAO6PG2G5TNKZK5TK3HG5ODR6DV6BANCNFSM4PMWKHKA .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OWASP/owasp-java-encoder/pull/37#issuecomment-666119678, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCMH36ZH26MXQXGDV23R6D627ANCNFSM4PMWKHKA.

-- Jim Manico Manicode Security https://www.manicode.com

jmanico commented 4 years ago

My mistake, I see you are indeed following previous patterns. Let me merge this and do a release as soon as we can.

On 7/30/20 1:16 AM, Kevin W. Wall wrote:

I just was following the pattern that you guys started and it already had a dependency on ESAPI in that shim.

Personally, I thought it would always have made more sense for your project to just put this out there as example code. If you don't want the dependency on ESAPI's DefaultEncoder, you could always just throw a RuntimeException of "Method not implemented" (although technically that would be breaking the interface contract).

Were it not for a few additional dependencies, it would make way more sense to do this from the ESAPI side. Maybe when we get to ESAPI 3 and I can abandon backward compatibility of the 2.0 interfaces. But today ESAPI already has too many dependencies.

But given that anyone who would be using this is already going to be using ESAPI, I don't think the dependency on ESAPI is that much of a big deal. (And if you're not using ESAPI already why would you ever use this instead of using the Java Encoder project directly?)

-kevin

Blog: http://off-the-wall-security.blogspot.com/ | Twitter: @KevinWWall NSA: All your crypto bit are belong to us.

On Thu, Jul 30, 2020, 00:00 Jim Manico notifications@github.com wrote:

Hey Kevin,

Does this bring in a dependency on ESAPI to the Encoder?

Why are we adding ESAPI code to the encoder as opposed to moving this ESAPI shell code the the ESAPI project?

-- Jim Manico @Manicode

On Jul 29, 2020, at 11:37 PM, Kevin W. Wall notifications@github.com wrote:

 Close #31

Note that you will need to do an official release for this. I did not update your owasp-java-encoder version because I wasn't sure if you wanted to call it 1.2.3 or 1.3.0 or what.

IMPORTANT NOTE Your minimal JDK is 1.5. As of ESAPI 2.2, or minimal supported JDK is 1.7. Please make your users aware of that should they choose to use this never version of org.owasp.encoder:encoder-esapi

You can view, comment on, or merge this pull request online at:

https://github.com/OWASP/owasp-java-encoder/pull/37

Commit Summary

Bump min ESAPI dependency from 2.0 to 2.2. Bring ESAPIEncoder in compliance with ESAPI 2.2.0.0 and later Encoder interface which added new methods. Minimal properties to get JUnit tests working for ESAPIEncoderTest. File Changes

M esapi/pom.xml (2) M esapi/src/main/java/org/owasp/encoder/esapi/ESAPIEncoder.java (14) M esapi/src/test/resources/.esapi/ESAPI.properties (39) Patch Links:

https://github.com/OWASP/owasp-java-encoder/pull/37.patch https://github.com/OWASP/owasp-java-encoder/pull/37.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub

https://github.com/OWASP/owasp-java-encoder/pull/37#issuecomment-666085771, or unsubscribe

https://github.com/notifications/unsubscribe-auth/AAO6PG2G5TNKZK5TK3HG5ODR6DV6BANCNFSM4PMWKHKA .

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OWASP/owasp-java-encoder/pull/37#issuecomment-666119678, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCMH36ZH26MXQXGDV23R6D627ANCNFSM4PMWKHKA.

-- Jim Manico Manicode Security https://www.manicode.com

jmanico commented 4 years ago

I got a travis build error can you  take a peek at this?

https://travis-ci.org/github/OWASP/owasp-java-encoder/builds/713156542

On 7/29/20 11:37 PM, Kevin W. Wall wrote:

Close #31 https://github.com/OWASP/owasp-java-encoder/issues/31

Note that you will need to do an official release for this. I did not update your owasp-java-encoder version because I wasn't sure if you wanted to call it 1.2.3 or 1.3.0 or what.

IMPORTANT NOTE Your minimal JDK is 1.5. As of ESAPI 2.2, or minimal supported JDK is 1.7. Please make your users aware of that should they choose to use this never version of org.owasp.encoder:encoder-esapi


    You can view, comment on, or merge this pull request online at:

  https://github.com/OWASP/owasp-java-encoder/pull/37

    Commit Summary

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/OWASP/owasp-java-encoder/pull/37, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCJIO6TMN5DHTWWSTVDR6DTH5ANCNFSM4PMWKHKA.

-- Jim Manico Manicode Security https://www.manicode.com

kwwall commented 4 years ago

Note that since owasp-java-encoder has no logger at all, it would be better to configure this to use JUL rather than SLF4J. That means if something goes wrong with testing, at least an actual error will be logged. Note that I had trouble getting this to actually build with this last commit (#d816d12) until I removed my Maven ~/.m2/repository. That may be why Travis build is failing.

jeremylong commented 4 years ago

@jmanico the build error was because oraclejdk is not available on newer travis images. See PR #40 which updated the .travis.yml to specify dist: trusty.

I'm assuming we should include this PR in release v1.2.3?

jmanico commented 4 years ago

Yes, please!

-- Jim Manico @Manicode

On Nov 7, 2020, at 3:12 AM, Jeremy Long notifications@github.com wrote:

3?