-
Notifications
You must be signed in to change notification settings - Fork 365
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
EncryptedPropertiesUtilsTest.testCreateNew:93 » UnsupportedOperation This method has been removed for security. #721
Comments
…SAPI#721) Several of ReferenceEncryptedProperties's methods were deprecated and now throw an UnsupportedOperation exception. This commit comments out the offending code and prints a message alerting to the removal.
The short answer is that all of these need to be rewritten with new versions of Power mock and mockito. Those frameworks utilize some reflection methods that have gone away. That’s why you’re getting the unsupported operation exceptions. |
Thanks @xeno6696,
Ok, thanks. I commented them out for the time being. It is easier to skip the test than it is to field questions about why they don't work. Also see
This might also be related to $ grep -IR 'This method has been removed for security'
src/main/java/org/owasp/esapi/reference/crypto/ReferenceEncryptedProperties.java: throw new UnsupportedOperationException("This method has been removed for security.");
src/main/java/org/owasp/esapi/reference/crypto/ReferenceEncryptedProperties.java: throw new UnsupportedOperationException("This method has been removed for security.");
src/main/java/org/owasp/esapi/reference/crypto/ReferenceEncryptedProperties.java: throw new UnsupportedOperationException("This method has been removed for security.");
src/main/java/org/owasp/esapi/reference/crypto/ReferenceEncryptedProperties.java: throw new UnsupportedOperationException("This method has been removed for security.");
src/main/java/org/owasp/esapi/reference/crypto/ReferenceEncryptedProperties.java: throw new UnsupportedOperationException("This method has been removed for security."); |
I admit that I didn't read into this or the PR very carefully. I've known for some time that all the mocking unit tests we started working with would eventually have to be redone, but what you're referencing here in ReferenceEncryptedProperties wasn't part of that. My comment might be totally off base in this instance, my apologies. |
… availabilty See KWW's comments in GH ESAPI#721.
On my Java 11 machine I pass the self tests. On GitHub with Java 8 (?) it is failing with: Error: Failures:
Error: ReferenceEncryptedPropertiesTest.testStoreLoadWithReader:274 Key one was never seen
[INFO]
Error: Tests run: 4274, Failures: 1, Errors: 0, Skipped: 0 The code in question is here. The line numbers are off a bit, but the reference file line number is 140. Would you take a look at what I am doing wrong with this PR? |
@noloader - Wait, what? You're saying if you run What does running ' Java... write once, debug everywhere. |
@noloader wrote:
Looking at the code, that makes sense. UnsupportedOperationException is an unchecked exception, so devs are not going catch it. And this has absolutely ZERO to do with PowerMock and/or Mockito, because the JUnit test ReferenceEncryptedPropertiesTest uses neither of those. But, if it is also failing for you on a Linux with Java 8, that would also seem to rule out any weirdness from reflection related things in the JDK. It seems to go far deeper than that. You didn't change the version of JUnit or any other dependencies, did you. I could see maybe switching up JUnit could cause weirdness like this, but short of that, I can't think of anything. |
I'm probably doing something wrong with the way I am testing if |
@noloader - So, please explain what you mean by "Patched with dynamic detection"? You mean in the test itself? If so, can you point to the specific line #s or the specific commit(s)? (I've not yet looked at your changes.) Note that I can confirm your test failures under Java 11. If I change my JDK to Java 11 and run the same test, I get the same failure that you do. And given the failure, I'm surprised that you are getting any of the tests to run successfully with Java 11. |
From https://github.com/ESAPI/esapi-java-legacy/pull/728/files : protected boolean hasSettersAndStores() {
ReferenceEncryptedProperties props = new ReferenceEncryptedProperties();
try {
props.setProperty("x", "y");
props.store(new ByteArrayOutputStream(), "XYZ");
}
catch (UnsupportedOperationException ex) {
return false;
}
catch (IOException ex) {
return false;
}
finally {
try {
props.remove("x");
props.remove("XYZ");
}
catch (Exception ex) {
}
}
return true;
} Then later, an existing test guarded by @Test public void testStoreLoad() throws Exception
{
if ( hasSettersAndStores() == false ) {
// https://github.com/ESAPI/esapi-java-legacy/issues/721
System.out.println("testStoreLoadWithReader removed due to deprecation of ReferenceEncryptedProperties.store()");
}
else {
//create an EncryptedProperties to store
ReferenceEncryptedProperties toStore = new ReferenceEncryptedProperties();
toStore.setProperty("one", "two");
toStore.setProperty("two", "three");
toStore.setProperty("seuss.schneier", "one fish, twofish, red fish, blowfish");
...
} |
Long ago, in a galaxy far, far away, @xeno6696 scribed thusly:
Looking at this more closely, it looks like the failure is org.apache.maven.plugin.MojoFailureException in the mave-surefire-plugin, so I'm going to take a stab and say maybe it has something to do with that plugin? First hit on Googling for 'java 11 maven-surefire-plugin' is https://stackoverflow.com/questions/53437819/maven-surefire-and-jdk-11 so seems as though we're not the only one. It also happens to mention a solution toward the end; namely adding this to the plugin's configuration: <argLine>
--illegal-access=permit
</argLine> @noloader - Maybe you can take a look and give it a try??? |
I wanted to forgo options like this. This makes folks RTFM:
Instead, I switched to a try/catch around the operation. This is compatible with all Java versions, and works with/without command line arguments: boolean supported = true;
try {
EncryptedPropertiesUtils.storeProperties(encryptedFilePath, props, "<property value>");
}
catch (UnsupportedOperationException ex) {
supported = false;
}
if ( supported ) {
// Continue with self test
}
else {
// Print message that test was skipped
} |
This workaround also fixes the 4 failed self tests with Java 13. |
Weird. I run on Linux Mint 19.2, and I don't get that error. I just check
our 'develop' branch (
https://github.com/ESAPI/esapi-java-legacy/blob/develop/src/test/java/org/owasp/esapi/reference/crypto/EncryptedPropertiesUtilsTest.java#L79)
and that method is still present. We do know you have to use Java 8 for
testing. It fails with Java 11.
Can you maybe attach the 2 Surefire dump files for this?
…-kevin
On Sat, Jul 16, 2022, 10:05 PM Jeffrey Walton ***@***.***> wrote:
Hi Everyone/Kevin,
I'm building ESAPI develop from GitHub on Ubuntu 20.04, x86_64, fully
patched.
mvn test is failing:
$ mvn test
...
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.994 s - in org.owasp.esapi.crypto.ESAPICryptoMACByPassTest
[INFO]
[INFO] Results:
[INFO]
[ERROR] Errors:
[ERROR] EncryptedPropertiesUtilsTest.testCreateNew:93 » UnsupportedOperation This method has been removed for security.
[ERROR] EncryptedPropertiesUtilsTest.testLoadEncryptedAndAdd:165 » UnsupportedOperation This method has been removed for security.
[ERROR] EncryptedPropertiesUtilsTest.testLoadPlaintextAndEncrypt:131 » UnsupportedOperation This method has been removed for security.
[ERROR] ReferenceEncryptedPropertiesTest.testStoreLoad:160 » UnsupportedOperation This method has been removed for security.
[INFO]
[ERROR] Tests run: 4276, Failures: 0, Errors: 4, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 33.429 s
[INFO] Finished at: 2022-07-16T22:01:31-04:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.0.0-M7:test (default-test) on project esapi:
[ERROR]
[ERROR] Please refer to /home/jwalton/Desktop/esapi-legacy-fork/target/surefire-reports for the individual test results.
[ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
Please let me know what else I can supply for you.
—
Reply to this email directly, view it on GitHub
<#721>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAO6PGYGY4SZGH5SC7RN7NLVUNS7RANCNFSM53Y65WRQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
One possibility maybe we can leverage the maven-surefire-plugim to allow
skipping of *specific* tests. Then you could just script that to ignore
what is failing for you.
That may require pom changes to allow that, but it is preferred over
marking tests as ignored because we need to see if things start failing.
-kevin
…On Tue, Jul 19, 2022, 3:21 PM Matt Seil ***@***.***> wrote:
The short answer is that all of these need to be rewritten with new
versions of Power mock and mockito.
Those frameworks utilize some reflection methods that have gone away.
That’s why you’re getting the unsupported operation exceptions.
—
Reply to this email directly, view it on GitHub
<#721 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAO6PG27KSI3MT4SS6GK5VDVU3535ANCNFSM53Y65WRQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi Everyone/Kevin,
I'm building ESAPI develop from GitHub on Ubuntu 20.04, x86_64, fully patched. I'm also seeing this on Fedora 36, x86_64, fully patched.
mvn test
is failing:Here is the surefire-reports files: surefire-reports.zip
Please let me know what else I can supply for you.
And here is the result of
mvn -e
:The text was updated successfully, but these errors were encountered: