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.

Sign up for GitHub

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

Jump to bottom

Unable to compile v1.2.3 branch with JDK8 #74

Closed
janikgithub opened this issue Jul 26, 2024 · 14 comments · Fixed by #76
Closed

Unable to compile v1.2.3 branch with JDK8 #74

janikgithub opened this issue Jul 26, 2024 · 14 comments · Fixed by #76
Assignees
@kwwall

Comments

@janikgithub
Copy link

I am building v1.2.3

bash-4.2$ javac -version
javac 1.8.0_412

bash-4.2$ mvn -v
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /scratch/apache-maven/apache-maven-3.6.3

mvn clean package

INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /scratch/gitroot/oswap/owasp-java-encoder/esapi/src/main/java/org/owasp/encoder/esapi/ESAPIEncoder.java:[131,13] org.owasp.encoder.esapi.ESAPIEncoder.Impl is not abstract and does not override abstract method decodeFromJSON(java.lang.String) in org.owasp.esapi.Encoder
[INFO] 1 error
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for OWASP Java Encoder Project 1.2.3:
[INFO] 
[INFO] OWASP Java Encoder Project ......................... SUCCESS [  8.897 s]
[INFO] Java Encoder ....................................... SUCCESS [ 25.773 s]
[INFO] JSP Encoder ........................................ SUCCESS [  3.623 s]
[INFO] ESAPI Thunk ........................................ FAILURE [  9.044 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  47.522 s
[INFO] Finished at: 2024-07-26T00:23:13Z
[INFO] ------------------------------------------------------------------------
@kwwall
Copy link
Contributor

kwwall commented Jul 26, 2024 •
edited
Loading

@jmanico - Since this is related to ESAPI Thunk, you can assign this issue to me and I'll create a PR for it. It probably just needs a newer version of ESAPI. I am unable to assign this GH issue to myself.

@kwwall
Copy link
Contributor

kwwall commented Jul 26, 2024 via email

@jeremylong
Copy link
Member

jeremylong commented Jul 26, 2024 •
edited
Loading

Would be as simple as adding:

        public String decodeFromJSON(String s) {
            throw new UnsupportedOperationException("OWASP Java Encoder does not support decoding");
        }
        
        public String encodeForJSON(String s) {
            // forJavaScriptSource(s) could be used instead.
            return Encode.forJavaScript(s);
        }

I'm just not sure if the ESAPI implementation expects the output to be quoted or not?

@jeremylong
Copy link
Member

If adding those two methods works - I can just push a PR. LMK

@janikgithub
Copy link
Author

Is there an ETA for the PR? Thanks

@kwwall
Copy link
Contributor

kwwall commented Jul 27, 2024 •
edited
Loading

@jeremylong - I think there's a few problems with with your proposal:

  1. The idea of ESAPI Thunk is so that developers an still use ESAPI, but with the Java Encoder Project rather than ESAPI's default Encoder (DefaultEncoder). However if you just throw an exception for decodeFromJSON that will be inconsistent with other places where you are deferring to the ESAPI decoder which will cause confusion.
  2. If you decide to make it consistent by changing your ESAPIEncoder calls to decodeForHTML, decodeFromURL, and decodeFromBase64 then that could potentially break your client's code.
  3. The base version of ESAPI that supports JSON encoding / decoding is 2.5.1.0. You'd have to at least change your minimum lower bound for the ESAPI version accepted from 2.2.3.1 to 2.5.1.0. (Or, if you want to only accept versions of ESAPI that use the new Jakarta Servlet API rather than the older JavaEE Servlet API, you need to make 2.5.3.0 the base line and possibly allow for the optional 'jarkarta' <classifier> to be passed down.

Note that we didn't make a big deal of keeping the same minor version # for this as best practice is to pin versions and ranges are discouraged. And 2.x has been around for so long and has had breaking changes (e.g., removing deprecating methods) and we generally reserve changing he minor # for breaking changes that are not 100% backward compatible. And since adding new features is backward compatible and we didn't bother to change the minor version # here. So, yeah, we have take some liberties with semantic versioning (although no more than Java's JDK itself), but I either forgot or wasn't aware that you were not using a pinned version of ESAPI.

jeremylong added a commit that referenced this issue Jul 28, 2024
@jeremylong
fix: update esapi
9a06b5c 
resolves #74
@jeremylong jeremylong mentioned this issue Jul 28, 2024
Merged
@jeremylong
Copy link
Member

@kwwall can you look at #76? Currently getting:

testEncode(org.owasp.encoder.esapi.ESAPIEncoderTest)  Time elapsed: 0.221 sec  <<< ERROR!
org.owasp.esapi.errors.ConfigurationException: java.lang.reflect.InvocationTargetException Encoder class (org.owasp.encoder.esapi.ESAPIEncoder) CTOR threw exception.
	at org.owasp.esapi.util.ObjFactory.make(ObjFactory.java:129)
	at org.owasp.esapi.ESAPI.encoder(ESAPI.java:101)
	at org.owasp.encoder.esapi.ESAPIEncoderTest.testEncode(ESAPIEncoderTest.java:26)
Caused by: java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.owasp.esapi.util.ObjFactory.make(ObjFactory.java:86)
	... 24 more
Caused by: java.lang.ExceptionInInitializerError
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:315)
	at org.owasp.esapi.util.ObjFactory.loadClassByStringName(ObjFactory.java:158)
	at org.owasp.esapi.util.ObjFactory.make(ObjFactory.java:81)
	at org.owasp.esapi.ESAPI.logFactory(ESAPI.java:139)
	at org.owasp.esapi.ESAPI.getLogger(ESAPI.java:155)
	at org.owasp.esapi.reference.DefaultEncoder.<init>(DefaultEncoder.java:85)
	at org.owasp.esapi.reference.DefaultEncoder.<init>(DefaultEncoder.java:109)
	at org.owasp.esapi.reference.DefaultEncoder.getInstance(DefaultEncoder.java:68)
	at org.owasp.encoder.esapi.ESAPIEncoder$Impl.<init>(ESAPIEncoder.java:141)
	at org.owasp.encoder.esapi.ESAPIEncoder$Impl.<clinit>(ESAPIEncoder.java:135)
	at org.owasp.encoder.esapi.ESAPIEncoder.getInstance(ESAPIEncoder.java:118)
	... 29 more
Caused by: org.owasp.esapi.errors.ConfigurationException: esapi-java-logging.properties is no longer supported.  See https://github.com/ESAPI/esapi-java-legacy/wiki/Configuring-the-JavaLogFactory for information on corrective actions.
	at org.owasp.esapi.logging.java.JavaLogFactory.<clinit>(JavaLogFactory.java:106)
	... 41 more

testSerialization(org.owasp.encoder.esapi.ESAPIEncoderTest)  Time elapsed: 0.001 sec  <<< ERROR!
org.owasp.esapi.errors.ConfigurationException: java.lang.reflect.InvocationTargetException Encoder class (org.owasp.encoder.esapi.ESAPIEncoder) CTOR threw exception.
	at org.owasp.esapi.util.ObjFactory.make(ObjFactory.java:129)
	at org.owasp.esapi.ESAPI.encoder(ESAPI.java:101)
	at org.owasp.encoder.esapi.ESAPIEncoderTest.testSerialization(ESAPIEncoderTest.java:34)
Caused by: java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.owasp.esapi.util.ObjFactory.make(ObjFactory.java:86)
	... 24 more
Caused by: java.lang.NoClassDefFoundError: Could not initialize class org.owasp.encoder.esapi.ESAPIEncoder$Impl
	at org.owasp.encoder.esapi.ESAPIEncoder.getInstance(ESAPIEncoder.java:118)
	... 29 more

@kwwall
Copy link
Contributor

kwwall commented Jul 28, 2024 via email

@janikgithub
Copy link
Author

Will this PR be merged into v1.2.3? Thanks

@jeremylong
Copy link
Member

no - we will be releasing 1.3.0 once the rest of the PRs are merged.

@janikgithub
Copy link
Author

What JDK will it support? Any ETA?
Thanks

@jeremylong
Copy link
Member

resulting jars will be Java 8 - but it will require Java 17 to build and test due to the required dependencies for the jakarta-jsp test cases.

@dwong77
Copy link

dwong77 commented Jul 30, 2024

What is the ETA for the 1.3.0 release? Thanks

@jeremylong
Copy link
Member

this week

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

@kwwall kwwall

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: update esapi OWASP/owasp-java-encoder
4 participants
@jeremylong @kwwall @janikgithub @dwong77