Transitive dependency on esapi
fransonsr opened this issue · 9 comments
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?
@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.
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.
@kwwall Just to clarify: I am merely an enthusiastic consumer, not an actual contributor. 😉
@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.
If this is still an issue please re-open.