-
Notifications
You must be signed in to change notification settings - Fork 154
Dlp #1909
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
Dlp #1909
Changes from 3 commits
bcc7dcb
497a43c
f0119d3
5f84eb6
e2536fe
6ebb9c4
9d2e722
699b226
7ffee70
559a62b
de3ed95
4c3ff8f
8cfb161
b17b14a
099ec70
02d75ec
2be464b
450204b
75b56d9
8adedb9
e7cc895
0f673bc
6a0bed1
469256e
53bc2f8
7d04071
ff15443
ea1e179
dce7806
e2eaa48
13711b1
7aa7e95
60a13df
0bf1268
21c3489
20ed318
c20c70d
528cc14
8d75f7c
c3ec071
e9a6286
8b1a278
77022dd
5008ee7
e58e11d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,109 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package com.predic8.membrane.core; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.fasterxml.jackson.core.JsonFactory; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.fasterxml.jackson.core.JsonParser; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.fasterxml.jackson.core.JsonToken; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import com.predic8.membrane.core.http.Message; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.io.InputStreamReader; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.*; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.logging.Logger; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public class DLP { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static final Logger logger = Logger.getLogger(DLP.class.getName()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final Map<String, String> riskDict; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static final JsonFactory JSON_FACTORY = new JsonFactory(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
russelmrcl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public DLP(Map<String, String> riskDict) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.riskDict = riskDict; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public Map<String, Object> analyze(Message msg) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return evaluateRisk(extractFieldNames(msg)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public Map<String, Object> analyze(Message msg) { | |
| return evaluateRisk(extractFieldNames(msg)); | |
| } | |
| public Map<String, Object> analyze(Message msg) { | |
| if (msg == null) { | |
| throw new IllegalArgumentException("Message cannot be null"); | |
| } | |
| return evaluateRisk(extractFieldNames(msg)); | |
| } |
🤖 Prompt for AI Agents
In core/src/main/java/com/predic8/membrane/core/DLP.java around lines 24 to 26,
the analyze method lacks null safety checks for the input Message parameter. Add
a null check at the start of the method to verify if msg is null, and handle
this case appropriately, such as returning an empty map or throwing an
IllegalArgumentException, to prevent NullPointerException during processing.
russelmrcl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Ensure proper resource management for JsonParser.
The JsonParser should be closed properly to avoid resource leaks. Consider using try-with-resources.
- try {
- JsonParser parser = JSON_FACTORY.createParser(new InputStreamReader(msg.getBodyAsStreamDecoded(), msg.getCharset()));
+ try (JsonParser parser = JSON_FACTORY.createParser(new InputStreamReader(msg.getBodyAsStreamDecoded(), msg.getCharset()))) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| JsonParser parser = JSON_FACTORY.createParser(new InputStreamReader(msg.getBodyAsStreamDecoded(), msg.getCharset())); | |
| try (JsonParser parser = JSON_FACTORY.createParser( | |
| new InputStreamReader(msg.getBodyAsStreamDecoded(), msg.getCharset()))) { | |
| // existing logic using parser |
🤖 Prompt for AI Agents
In core/src/main/java/com/predic8/membrane/core/DLP.java at line 31, the
JsonParser instance created is not closed, which can cause resource leaks.
Refactor the code to use a try-with-resources statement to automatically close
the JsonParser after use, ensuring proper resource management.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix array handling in JSON parsing logic.
The current implementation treats arrays the same as objects, which can lead to incorrect path building. Arrays don't have field names, so pushing currentFieldName onto the context stack for arrays may create malformed paths.
Consider this problematic JSON:
{
"users": [
{"name": "John"},
{"name": "Jane"}
]
}The current logic would incorrectly handle the array structure. Here's a suggested fix:
switch (token) {
case START_OBJECT:
- case START_ARRAY:
if (currentFieldName != null) {
contextStack.push(currentFieldName);
currentFieldName = null;
}
break;
+ case START_ARRAY:
+ if (currentFieldName != null) {
+ // For arrays, we want to include the array field name in the path
+ // but not treat array indices as field names
+ contextStack.push(currentFieldName);
+ currentFieldName = null;
+ }
+ break;
case END_OBJECT:
case END_ARRAY:🤖 Prompt for AI Agents
In core/src/main/java/com/predic8/membrane/core/DLP.java between lines 38 and
61, the JSON parsing logic incorrectly pushes the currentFieldName onto the
contextStack for both objects and arrays, which causes malformed paths since
arrays do not have field names. To fix this, modify the switch case so that only
START_OBJECT tokens push currentFieldName onto the contextStack, while
START_ARRAY tokens do not. This ensures that array contexts are handled
correctly without adding invalid field names to the path.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve exception handling strategy.
Throwing a RuntimeException for JSON parsing errors may not be the best approach. Consider returning an empty set, using a more specific exception type, or allowing the IOException to propagate to let callers handle it appropriately.
- } catch (IOException e) {
- throw new RuntimeException("Error extracting JSON field names", e);
- }
+ } catch (IOException e) {
+ logger.warning("Failed to parse JSON content: " + e.getMessage());
+ return new HashSet<>(); // Return empty set for malformed JSON
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (IOException e) { | |
| throw new RuntimeException("Error extracting JSON field names", e); | |
| } | |
| } catch (IOException e) { | |
| logger.warning("Failed to parse JSON content: " + e.getMessage()); | |
| return new HashSet<>(); // Return empty set for malformed JSON | |
| } |
🤖 Prompt for AI Agents
In core/src/main/java/com/predic8/membrane/core/DLP.java around lines 63 to 65,
the current code catches IOException and throws a RuntimeException, which is not
ideal for JSON parsing errors. Modify the exception handling to either return an
empty set when an IOException occurs or declare the method to throw IOException
so the caller can handle it properly. Avoid wrapping the IOException in a
RuntimeException to improve error handling clarity.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add input validation and improve exception handling.
The method lacks validation for null inputs and the message content type. Additionally, the generic RuntimeException could be more specific.
private Set<String> extractFieldNames(Message msg) {
+ if (msg == null) {
+ throw new IllegalArgumentException("Message cannot be null");
+ }
+
Set<String> fieldNames = new HashSet<>();
try {
JsonParser parser = JSON_FACTORY.createParser(new InputStreamReader(msg.getBodyAsStreamDecoded(), msg.getCharset()));
// ... existing parsing logic
} catch (IOException e) {
- throw new RuntimeException("Error extracting JSON field names", e);
+ throw new IllegalStateException("Failed to parse JSON content", e);
}
return fieldNames;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Set<String> extractFieldNames(Message msg) { | |
| Set<String> fieldNames = new HashSet<>(); | |
| try { | |
| JsonParser parser = JSON_FACTORY.createParser(new InputStreamReader(msg.getBodyAsStreamDecoded(), msg.getCharset())); | |
| Deque<String> contextStack = new ArrayDeque<>(); | |
| String currentFieldName = null; | |
| while (parser.nextToken() != null) { | |
| JsonToken token = parser.getCurrentToken(); | |
| switch (token) { | |
| case START_OBJECT: | |
| case START_ARRAY: | |
| if (currentFieldName != null) { | |
| contextStack.push(currentFieldName); | |
| currentFieldName = null; | |
| } | |
| break; | |
| case END_OBJECT: | |
| case END_ARRAY: | |
| if (!contextStack.isEmpty()) { | |
| contextStack.pop(); | |
| } | |
| break; | |
| case FIELD_NAME: | |
| currentFieldName = parser.currentName(); | |
| fieldNames.add(buildFullPath(contextStack, currentFieldName).toLowerCase()); | |
| break; | |
| default: | |
| break; | |
| } | |
| } | |
| } catch (IOException e) { | |
| throw new RuntimeException("Error extracting JSON field names", e); | |
| } | |
| return fieldNames; | |
| } | |
| private Set<String> extractFieldNames(Message msg) { | |
| if (msg == null) { | |
| throw new IllegalArgumentException("Message cannot be null"); | |
| } | |
| Set<String> fieldNames = new HashSet<>(); | |
| try { | |
| JsonParser parser = JSON_FACTORY.createParser( | |
| new InputStreamReader(msg.getBodyAsStreamDecoded(), msg.getCharset())); | |
| Deque<String> contextStack = new ArrayDeque<>(); | |
| String currentFieldName = null; | |
| while (parser.nextToken() != null) { | |
| JsonToken token = parser.getCurrentToken(); | |
| switch (token) { | |
| case START_OBJECT: | |
| case START_ARRAY: | |
| if (currentFieldName != null) { | |
| contextStack.push(currentFieldName); | |
| currentFieldName = null; | |
| } | |
| break; | |
| case END_OBJECT: | |
| case END_ARRAY: | |
| if (!contextStack.isEmpty()) { | |
| contextStack.pop(); | |
| } | |
| break; | |
| case FIELD_NAME: | |
| currentFieldName = parser.currentName(); | |
| fieldNames.add(buildFullPath(contextStack, currentFieldName).toLowerCase()); | |
| break; | |
| default: | |
| break; | |
| } | |
| } | |
| } catch (IOException e) { | |
| throw new IllegalStateException("Failed to parse JSON content", e); | |
| } | |
| return fieldNames; | |
| } |
🤖 Prompt for AI Agents
In core/src/main/java/com/predic8/membrane/core/DLP.java between lines 28 and
67, add validation to check if the input Message object is null and verify that
its content type is JSON before processing. Replace the generic RuntimeException
with a more specific exception, such as IOException or a custom exception, to
better indicate JSON parsing errors. Ensure that these validations and exception
handling improve robustness and clarity of error reporting.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Ensure consistent case handling between field extraction and dictionary lookup.
Field names are normalized to lowercase during extraction (line 56), but the risk dictionary lookup uses the field as-is. This could lead to case-sensitivity issues if the risk dictionary contains mixed-case keys.
- String risk = riskDict.getOrDefault(field, "unclassified");
+ String risk = riskDict.getOrDefault(field.toLowerCase(), "unclassified");Alternatively, normalize the risk dictionary keys during construction:
public DLP(Map<String, String> riskDict) {
- this.riskDict = riskDict;
+ this.riskDict = new HashMap<>();
+ riskDict.forEach((key, value) -> this.riskDict.put(key.toLowerCase(), value));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String risk = riskDict.getOrDefault(field, "unclassified"); | |
| String risk = riskDict.getOrDefault(field.toLowerCase(), "unclassified"); |
🤖 Prompt for AI Agents
In core/src/main/java/com/predic8/membrane/core/DLP.java at line 84, the risk
dictionary lookup uses the field variable without normalizing its case, while
field names are converted to lowercase during extraction at line 56. To fix
this, convert the field variable to lowercase before using it as a key in
riskDict.getOrDefault to ensure consistent case handling and avoid lookup
failures due to case differences.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix case sensitivity inconsistency.
Field names are normalized to lowercase during extraction (line 56), but risk dictionary values are compared using toLowerCase() without normalizing the dictionary keys. This creates an inconsistency where the dictionary lookup uses original casing but the switch uses lowercase.
- switch (risk.toLowerCase()) {
+ switch (risk) {Additionally, consider normalizing the risk dictionary keys in the constructor:
public DLP(Map<String, String> riskDict) {
- this.riskDict = riskDict;
+ this.riskDict = new HashMap<>();
+ riskDict.forEach((key, value) -> this.riskDict.put(key.toLowerCase(), value));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| switch (risk.toLowerCase()) { | |
| public class DLP { | |
| private final Map<String, String> riskDict; | |
| public DLP(Map<String, String> riskDict) { | |
| // normalize keys to lowercase so we can compare risk values directly | |
| this.riskDict = new HashMap<>(); | |
| riskDict.forEach((key, value) -> this.riskDict.put(key.toLowerCase(), value)); | |
| } | |
| // … | |
| private void evaluateRisk(String risk) { | |
| // risk is already normalized to lowercase earlier, so no need to call toLowerCase() here | |
| switch (risk) { | |
| case "low": | |
| // … | |
| break; | |
| case "medium": | |
| // … | |
| break; | |
| case "high": | |
| // … | |
| break; | |
| default: | |
| // … | |
| } | |
| } | |
| // … | |
| } |
🤖 Prompt for AI Agents
In core/src/main/java/com/predic8/membrane/core/DLP.java at line 87, the switch
statement uses risk.toLowerCase() but the risk dictionary keys are not
normalized to lowercase, causing case sensitivity issues. To fix this, normalize
all keys in the risk dictionary to lowercase in the constructor so lookups are
consistent. Then ensure the switch statement uses the lowercase risk value to
match the normalized keys.
russelmrcl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,64 @@ | ||||||||||
| package com.predic8.membrane.core; | ||||||||||
|
|
||||||||||
| import com.predic8.membrane.core.http.Message; | ||||||||||
| import org.junit.jupiter.api.BeforeEach; | ||||||||||
| import org.junit.jupiter.api.Test; | ||||||||||
|
|
||||||||||
| import java.io.ByteArrayInputStream; | ||||||||||
| import java.nio.charset.StandardCharsets; | ||||||||||
| import java.util.HashMap; | ||||||||||
| import java.util.Map; | ||||||||||
|
|
||||||||||
| import static org.junit.jupiter.api.Assertions.*; | ||||||||||
| import static org.mockito.Mockito.*; | ||||||||||
|
|
||||||||||
| class DLPTest { | ||||||||||
|
|
||||||||||
| private Message mockMessage; | ||||||||||
| private Map<String, String> riskDict; | ||||||||||
| String json = """ | ||||||||||
| { | ||||||||||
| "user": { | ||||||||||
| "email": "[email protected]", | ||||||||||
| "profile": { | ||||||||||
| "firstName": "John", | ||||||||||
| "lastName": "Doe" | ||||||||||
| } | ||||||||||
| }, | ||||||||||
| "active": true | ||||||||||
| } | ||||||||||
| """; | ||||||||||
|
|
||||||||||
| @BeforeEach | ||||||||||
| void setUp() { | ||||||||||
| mockMessage = mock(Message.class); | ||||||||||
| when(mockMessage.getBodyAsStreamDecoded()) | ||||||||||
| .thenReturn(new ByteArrayInputStream(json.getBytes(StandardCharsets.UTF_8))); | ||||||||||
| when(mockMessage.getCharset()) | ||||||||||
| .thenReturn(String.valueOf(StandardCharsets.UTF_8)); | ||||||||||
|
||||||||||
| when(mockMessage.getCharset()) | |
| .thenReturn(String.valueOf(StandardCharsets.UTF_8)); | |
| when(mockMessage.getCharset()) | |
| .thenReturn(StandardCharsets.UTF_8.name()); |
🤖 Prompt for AI Agents
In core/src/test/java/com/predic8/membrane/core/DLPTest.java around lines 37 to
38, the mocking of getCharset() returns String.valueOf(StandardCharsets.UTF_8),
which gives the charset object's string representation instead of the charset
name. Fix this by returning StandardCharsets.UTF_8.name() to provide the correct
charset name string expected by getCharset().
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add edge case testing.
The current test only covers the happy path. Consider adding tests for error scenarios and edge cases.
Add these additional test methods:
@Test
void analyzeWithEmptyJson() {
when(mockMessage.getBodyAsStreamDecoded())
.thenReturn(new ByteArrayInputStream("{}".getBytes(StandardCharsets.UTF_8)));
DLP dlp = new DLP(riskDict);
Map<String, Object> result = dlp.analyze(mockMessage);
assertEquals(0, result.get("high_risk"));
assertEquals(0, result.get("medium_risk"));
assertEquals(0, result.get("low_risk"));
assertEquals(0, result.get("unclassified"));
}
@Test
void analyzeWithInvalidJson() {
when(mockMessage.getBodyAsStreamDecoded())
.thenReturn(new ByteArrayInputStream("invalid json".getBytes(StandardCharsets.UTF_8)));
DLP dlp = new DLP(riskDict);
assertThrows(RuntimeException.class, () -> dlp.analyze(mockMessage));
}
@Test
void analyzeWithNullMessage() {
DLP dlp = new DLP(riskDict);
assertThrows(IllegalArgumentException.class, () -> dlp.analyze(null));
}🤖 Prompt for AI Agents
In core/src/test/java/com/predic8/membrane/core/DLPTest.java around lines 48 to
63, the analyze() test only covers the normal case. Add new test methods to
cover edge cases: one that tests analyze() with an empty JSON string and asserts
all risk counts are zero, another that tests analyze() with invalid JSON and
asserts it throws a RuntimeException, and a third that tests analyze() with a
null message and asserts it throws an IllegalArgumentException. Mock the message
body appropriately for these tests.
🛠️ Refactor suggestion
Expand test coverage for edge cases.
The current test only covers the happy path scenario. Consider adding tests for edge cases such as:
- Empty JSON objects
- Malformed JSON
- Null message input
- Empty risk dictionary
- Case sensitivity scenarios
Would you like me to generate additional test methods to cover these edge cases?
🤖 Prompt for AI Agents
In core/src/test/java/com/predic8/membrane/core/DLPTest.java around lines 48 to
63, the analyze() test only covers the happy path. Add new test methods to cover
edge cases including empty JSON objects, malformed JSON input, null message
input, empty risk dictionary, and case sensitivity scenarios. Each test should
create appropriate inputs to simulate these conditions and assert that the
analyze method handles them gracefully without errors or unexpected results.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/main/resources/dlp-fields.csv
DLPIntercept
init() load fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider making the risk dictionary case-insensitive.
The risk dictionary lookup uses exact string matching, but field names are normalized to lowercase (line 56). This could cause mismatches if the dictionary contains mixed-case keys.
Consider normalizing dictionary keys in the constructor:
public DLP(Map<String, String> riskDict) { + this.riskDict = new HashMap<>(); + riskDict.forEach((key, value) -> this.riskDict.put(key.toLowerCase(), value)); - this.riskDict = riskDict; }📝 Committable suggestion
🤖 Prompt for AI Agents