-
Notifications
You must be signed in to change notification settings - Fork 113
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
Changes to get owasp-java-encoder to work with ESAPI 2.2.0.0 and later #37
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 ***@***.***> 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:
#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.
|
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 ***@***.***> 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 ***@***.***>
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:
>
> #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
<#37 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAO6PG2G5TNKZK5TK3HG5ODR6DV6BANCNFSM4PMWKHKA>
.
|
I'm teaching today but will look into this tonight!
- Jim
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 ***@***.***> 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 ***@***.***>
> 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:
> >
> > #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
>
<#37 (comment)>,
> 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
<#37 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEBYCMH36ZH26MXQXGDV23R6D627ANCNFSM4PMWKHKA>.
--
Jim Manico
Manicode Security
https://www.manicode.com
|
My mistake, I see you are indeed following previous patterns. Let me
merge this and do a release as soon as we can.
- Jim
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 ***@***.***> 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 ***@***.***>
> 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:
> >
> > #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
>
<#37 (comment)>,
> 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
<#37 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEBYCMH36ZH26MXQXGDV23R6D627ANCNFSM4PMWKHKA>.
--
Jim Manico
Manicode Security
https://www.manicode.com
|
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 <#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:
#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
<https://github.com/OWASP/owasp-java-encoder/pull/37/files#diff-3c86c54a163b3eff33afc390fff76f61>
(2)
* *M* esapi/src/main/java/org/owasp/encoder/esapi/ESAPIEncoder.java
<https://github.com/OWASP/owasp-java-encoder/pull/37/files#diff-558845777ce2716f1b044cf53a2a3227>
(14)
* *M* esapi/src/test/resources/.esapi/ESAPI.properties
<https://github.com/OWASP/owasp-java-encoder/pull/37/files#diff-8faa16de1cce19693aa5d75054ab988e>
(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
<#37>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEBYCJIO6TMN5DHTWWSTVDR6DTH5ANCNFSM4PMWKHKA>.
--
Jim Manico
Manicode Security
https://www.manicode.com
|
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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