Added basic validation to Fontus' configuration. #15
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
It is fairly easy to leave old clutter in your configuration which will
in some cases fail more or less silently.
For example, I just copied a config from one of the tested programs for
the CCS paper and it contained the following converter:
```xml
<converter>
<opcode>184</opcode>
<owner>de/tubs/cs/ias/asm_test/taintaware/shared/IASStringUtils</owner>
<name>convertTStringList</name>
<descriptor>(Ljava/util/List;)Ljava/util/List;</descriptor>
<interface>false</interface>
</converter>
```
This predates the name change which is like 2y+ old. In case the
converter was actually applied, this would try to call a method that
does not exist anymore.
This patch aims to fix this by trying to assess whether the converters
are actually in the class path. It only prints out warnings, as failing
would be a) a breaking change b) prevent building a generic
configuration that works across programs.
The latter is due to the fact, that we ship some converters which rely
on the presence of third party libraries, such as
[DGMMethodConverter](https://github.com/SAP/project-fontus/blob/main/fontus/src/main/java/com/sap/fontus/manual/groovy/converters/DGMMethodConverter.java).
It is also included in most Fontus configs I have on my system, so it's
probably fairly widespread. I'd like to avoid breaking all of them, so
this simply warns if you define converters which can not be applied
without error.
Example output:
```txt
java.lang.ClassNotFoundException: org.codehaus.groovy.reflection.GeneratedMetaMethod$DgmMethodRecord
at com.sap.fontus.manual.groovy.converters.DGMMethodConverter.<clinit>(DGMMethodConverter.java:20)
at java.base/java.lang.Class.forName0(Native Method)
at java.base/java.lang.Class.forName(Class.java:315)
at com.sap.fontus.asm.FunctionCall.toMethod(FunctionCall.java:110)
at com.sap.fontus.config.Configuration.validate(Configuration.java:660)
at com.sap.fontus.agent.TaintAgent.premain(TaintAgent.java:16)
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 java.instrument/sun.instrument.InstrumentationImpl.loadClassAndStartAgent(InstrumentationImpl.java:513)
at java.instrument/sun.instrument.InstrumentationImpl.loadClassAndCallPremain(InstrumentationImpl.java:525)
Converter 'convertTypes' is invalid due to: java.lang.ClassNotFoundException: org.codehaus.groovy.reflection.GeneratedMetaMethod$DgmMethodRecord
Converter 'convertStringList' is invalid due to: de.tubs.cs.ias.asm_test.taintaware.shared.IASStringUtils
Converter 'convertTStringList' is invalid due to: de.tubs.cs.ias.asm_test.taintaware.shared.IASStringUtils
Converter 'convertTStringToTStringHashTable' is invalid due to: de.tubs.cs.ias.asm_test.taintaware.shared.IASStringUtils
Converter 'convertStringMapToTStringMap' is invalid due to: de.tubs.cs.ias.asm_test.taintaware.shared.IASStringUtils
Converter 'convertStringHashtableToTStringHashtable' is invalid due to: de.tubs.cs.ias.asm_test.taintaware.shared.IASStringUtils
```
The stacktrace originates from the [DGMMethodConverter](https://github.com/SAP/project-fontus/blob/main/fontus/src/main/java/com/sap/fontus/manual/groovy/converters/DGMMethodConverter.java#L26), I'll clean that up separately.
tmbrbr
approved these changes
Feb 23, 2024
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.
First step towards #14.
It is fairly easy to leave old clutter in your configuration, which will, in some cases, fail more or less silently.
For example, I just copied a config from one of the tested programs for the CCS paper, and it contained the following converter:
This predates the name change, which is like 2y+ old. In case the converter was actually applied, this would try to call a method that does not exist anymore.
This patch aims to fix this by trying to assess whether the converters are actually in the classpath. It only prints out warnings, as failing would be a) a breaking change b) prevent building a generic configuration that works across programs.
The latter is due to the fact that we ship some converters which rely on the presence of third-party libraries, such as
DGMMethodConverter. It is also included in most Fontus configs I have on my system, so it's probably fairly widespread. I'd like to avoid breaking all of them, so this simply warns if you define converters that can not be applied without error.
Example output:
The stack trace originates from the DGMMethodConverter, I'll clean that up separately.