Skip to content
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

Transitive dependency on esapi #30

Closed
fransonsr opened this issue Apr 30, 2019 · 9 comments
Closed

Transitive dependency on esapi #30

fransonsr opened this issue Apr 30, 2019 · 9 comments

Comments

@fransonsr
Copy link

The transitive dependency on esapi today picked up version 2.2.0.0-RC2 which was also released today. Unfortunately, there is a bug that broke our code in that version. With the "principle of least astonishment" I would not have expected to pick up a release-candidate build. Should not the dependency be limited to actual releases?

@kwwall
Copy link
Contributor

kwwall commented Apr 30, 2019

@fransonsr I fear that we broke more than just the ESAPI GitHub Issue #488 that you referenced.I just pulled down your current code base and tried to compile it and got compilation errors:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.5.1:compile (default-compile) on project encoder-esapi: Compilation failure
[ERROR] /home/kww/Code/GitHub/owasp-java-encoder/esapi/src/main/java/org/owasp/encoder/esapi/ESAPIEncoder.java:[128,13] org.owasp.encoder.esapi.ESAPIEncoder.Impl is not abstract and does not override abstract method getCanonicalizedURI(java.net.URI) in org.owasp.esapi.Encoder

And I suspect that there are a lot more subtle things as well. For instance, we previously compiled ESAPI with '-target 1.6', but starting with 2.2.0.0-RC1 (which was never pushed to Maven Central), we changed that to '-target 1.7'. But your README.md states that OWASP Java Encoder will work with JDK 1.5. Not if it is used in this mode it won't.

I'm not really sure what the whole idea / purpose of this "encoder-esapi" (aka, "ESAPI Thunk") is anyway. If it really is, as your esapi/pom.xml documentation states to "provides an easy way to plugin the Encoder Projects API into an implementation of ESAPI", I think it would be a lot more straightforward to simply tweak one's ESAPI.properties "ESAPI.Encoder" property with some appropriate OWASP Java Encoder class and work things that way. (I.e., take a path similar to what I did with getting ESAPI to work with the OWASP Java HTML Sanitizer project instead of using OWASP AntiSamy.) I think that what you have is likely to be way too brittle and @xeno6696 and I can't promise you that this will work without problems now or in the future. (For one thing, unlike Sun and Oracle, when we deprecate things, we eventually go back and actually remove them at some reasonable time!) However, if you want to work in the other direction as I described above, we probably can try to support that.

@kwwall
Copy link
Contributor

kwwall commented Apr 30, 2019

And BTW, while we are on the topic of compatibility, Matt Seil fixed how ESAPI encoders were handling non-BMP characters in ESAPI GitHub Issue #300. So that fix is going to introduce a likely discrepancy between the reference ESAPI encoder and the OWASP Java Encoder Project. So I would highly encourage you to look at those fixes for the Java Encoder project.

@fransonsr
Copy link
Author

@kwwall Just to clarify: I am merely an enthusiastic consumer, not an actual contributor. 😉

@kwwall
Copy link
Contributor

kwwall commented Apr 30, 2019 via email

@jmanico
Copy link
Member

jmanico commented Apr 30, 2019 via email

@kwwall
Copy link
Contributor

kwwall commented Apr 30, 2019

@manicode @jmanico (sorry for the dups; not sure which of these you are using) -- Not a "question" so much as a "concern". Looks like ESAPI 2.2.0.0-RC2 release broke things more than just the way that this issue implies. See my earlier comments in this thread for the compilation errors it caused. If there is something that you need the ESAPI team to do as a result, then please create a GitHub issue for us. Thanks.

@jmanico
Copy link
Member

jmanico commented Apr 30, 2019 via email

@jmanico
Copy link
Member

jmanico commented Apr 30, 2019 via email

@jmanico
Copy link
Member

jmanico commented Jul 24, 2020

If this is still an issue please re-open.

@jmanico jmanico closed this as completed Jul 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants