[TE-27807] Nlp to fetch the value of request headers from network logs. #199
Conversation
|
|
WalkthroughThis change introduces a new Java project, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StartTracking (Action)
participant Selenium BiDi Network
participant ResponseDataUtilities
participant FileUtilities
User->>StartTracking (Action): Execute action with URL
StartTracking (Action)->>Selenium BiDi Network: Add listener for onBeforeRequestSent
Selenium BiDi Network-->>StartTracking (Action): Event with request data
StartTracking (Action)->>ResponseDataUtilities: saveAllNetworkData(runId, headers)
ResponseDataUtilities->>FileUtilities: writeToFile(runId, encodedHeaders)
FileUtilities-->>ResponseDataUtilities: Acknowledge
ResponseDataUtilities-->>StartTracking (Action): Acknowledge
StartTracking (Action)-->>User: Action result (success/failure)
sequenceDiagram
participant User
participant GetDataFromRequestBody (Action)
participant ResponseDataUtilities
participant FileUtilities
User->>GetDataFromRequestBody (Action): Execute action with header key
GetDataFromRequestBody (Action)->>ResponseDataUtilities: getSpecificHeaderValue(runId, headerKey)
ResponseDataUtilities->>FileUtilities: readFromFile(runId)
FileUtilities-->>ResponseDataUtilities: Return encodedHeaders
ResponseDataUtilities-->>GetDataFromRequestBody (Action): Return header value
GetDataFromRequestBody (Action)-->>User: Store value in runtime variable, return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java (4)
23-23: Fix grammar error in log message.- logger.info("Did not saved data for the current testcase " + e.getMessage()); + logger.info("Did not save data for the current testcase " + e.getMessage());
47-56: Simplify control flow in getRequestHeadersData method.The current logic has an unnecessary path - if data exists but doesn't have "requestHeaders", it logs a message then continues to throw an exception. This can be simplified.
} catch (JsonParseException e) { clearResponseDataByRunId(runId); logger.info("Failed to parse request headers data for runId: " + runId + " with exception: " + e.getMessage()); throw new Exception(e); } - logger.info("Failed to get any request headers data for runId: " + runId); - throw new Exception("Failed to get any request headers data for runId: " + runId); + return new JsonArray(); // Return empty array if no request headers found
72-72: Consider using explicit type instead of 'var' for consistency.While 'var' is valid Java syntax, using explicit types may be more consistent with the rest of the codebase and improve readability.
- var headerElement = headersArray.get(0); + JsonElement headerElement = headersArray.get(0);
88-111: Consider removing legacy string format support if not needed.The fallback logic for string format headers (lines 88-111) adds significant complexity. If this legacy support is not required, consider removing it to simplify the code.
If legacy support is needed, consider extracting this logic into a separate private method:
private static String parseHeaderFromString(String headersString, String headerKey, Logger logger) { // Move the string parsing logic here }store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/GetDataFromRequestBody.java (2)
23-27: Remove unused test data fields.The
urlValueandmethodValuefields are declared but never used in theexecute()method. If they're not needed, they should be removed to avoid confusion.- @TestData(reference = "url_value") - private com.testsigma.sdk.TestData urlValue; - - @TestData(reference = "method_value") - private com.testsigma.sdk.TestData methodValue;If these fields are intended for future use or validation, add TODO comments explaining their purpose.
43-43: Fix typo in log message.- logger.info("Execution started for action: GetDataFromrequestBody"); + logger.info("Execution started for action: GetDataFromRequestBody");store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/utilities/FileUtilities.java (2)
29-54: Simplify writeToFile method using FileUtils built-in capabilities.The current implementation manually handles directory creation and file deletion. FileUtils provides more robust methods that can handle this automatically.
public static void writeToFile(Long runId, String content) throws Exception { String fileName = getFileName(runId); File file = new File(fileName); try { - // Ensure the parent directory exists without overwriting it - File parentDir = file.getParentFile(); - if (parentDir != null && !parentDir.exists()) { - if (!parentDir.mkdirs()) { - throw new IOException("Failed to create directory: " + parentDir); - } - } - - // Check if file exists, delete if it does - if (file.exists()) { - if (!file.delete()) { - throw new IOException("Failed to delete existing file: " + file); - } - } - // Write the content to the file - FileUtils.writeStringToFile(file, content, StandardCharsets.UTF_8); - // Write the JSON content to the file - FileUtils.writeStringToFile(file, content, StandardCharsets.UTF_8); + FileUtils.writeStringToFile(file, content, StandardCharsets.UTF_8); } catch (IOException e) { throw new Exception(e); } }
64-64: Use logging framework instead of System.out.println.For consistency with the rest of the application, consider using a logging framework instead of direct console output.
If a logger is not available in this utility class, consider removing this log statement entirely or adding a logger parameter to the method.
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/StartTracking.java (2)
57-72: Simplify header processing logic.The current approach of maintaining separate lists for header names and values works but can be simplified and made more robust.
Consider this cleaner approach:
-// create a list of headers and add the header name and header value to the list -List<String> headerNameList = new ArrayList<>(); -List<String> headerValueList = new ArrayList<>(); -for (Header header : beforeRequestSent.getRequest().getHeaders()) { - String headerValue = header.getValue().getType() == BytesValue.Type.STRING - ? header.getValue().getValue().toString() - : new String(header.getValue().getValue()); - logger.info(header.getName() + ": " + headerValue); - headerNameList.add(header.getName()); - headerValueList.add(headerValue); -} -logger.info("Adding request body data using header names and values..."); -// add request headers as json array -JsonObject requestHeaders = new JsonObject(); -for (int i = 0; i < headerNameList.size(); i++) { - requestHeaders.addProperty(headerNameList.get(i), headerValueList.get(i)); -} +// Process headers directly into JSON object +JsonObject requestHeaders = new JsonObject(); +for (Header header : beforeRequestSent.getRequest().getHeaders()) { + String headerValue = header.getValue().getType() == BytesValue.Type.STRING + ? header.getValue().getValue().toString() + : new String(header.getValue().getValue()); + logger.info(header.getName() + ": " + headerValue); + requestHeaders.addProperty(header.getName(), headerValue); +} +logger.info("Adding request body data using header names and values...");
80-87: Error handling is functional but could be cleaner.The nested try-catch blocks handle errors appropriately by allowing the network listener to continue working even if individual requests fail. However, the deep nesting reduces readability.
Consider extracting the header processing logic into a separate method:
+private void processRequestHeaders(BeforeRequestSent beforeRequestSent, String targetUrl) { + try { + String interceptedUrl = beforeRequestSent.getRequest().getUrl(); + logger.info("Intercepted: " + interceptedUrl); + + if (interceptedUrl.contains(targetUrl)) { + logger.info("\n=== MATCHED TARGET URL ==="); + logger.info("Full URL: " + interceptedUrl); + logger.info("\nRequest Headers:"); + + // Process headers and save data + JsonObject requestHeaders = extractHeaders(beforeRequestSent); + saveNetworkData(requestHeaders); + + logger.info("=== END OF MATCHED REQUEST ===\n"); + } + } catch (Exception e) { + logger.debug("Error processing request: " + e.getMessage()); + } +}Then the main listener becomes:
network.onBeforeRequestSent(beforeRequestSent -> { - try { - // ... entire processing logic - } catch (Exception e) { - logger.debug("Error processing request: " + e.getMessage()); - } + processRequestHeaders(beforeRequestSent, targetUrl); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
store_network_logs_using_bidi/pom.xml(1 hunks)store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/GetDataFromRequestBody.java(1 hunks)store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/StartTracking.java(1 hunks)store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/utilities/FileUtilities.java(1 hunks)store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java(1 hunks)store_network_logs_using_bidi/src/main/resources/testsigma-sdk.properties(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java (1)
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/utilities/FileUtilities.java (1)
FileUtilities(10-71)
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/GetDataFromRequestBody.java (2)
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/StartTracking.java (1)
Data(21-98)store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java (1)
ResponseDataUtilities(12-116)
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/StartTracking.java (2)
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/GetDataFromRequestBody.java (1)
Data(15-79)store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/utilities/ResponseDataUtilities.java (1)
ResponseDataUtilities(12-116)
🪛 Gitleaks (8.27.2)
store_network_logs_using_bidi/src/main/resources/testsigma-sdk.properties
1-1: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🔇 Additional comments (7)
store_network_logs_using_bidi/pom.xml (1)
1-106: LGTM! Well-structured Maven configuration.The POM file is properly configured with:
- Appropriate Java 11 compatibility settings
- Well-defined dependencies with explicit versions
- Correct plugin configuration for creating uber-jar with Maven Shade Plugin
- Source plugin configuration for attaching source jars
The dependency versions appear reasonable and should work well together for this TestSigma addon project.
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/StartTracking.java (6)
1-19: LGTM! Clean imports and package structure.The imports are well-organized and include all necessary dependencies for Selenium BiDi network interception, JSON handling, and Testsigma SDK integration.
21-25: LGTM! Proper action configuration.The annotations are correctly configured with descriptive action text and clear description that explains the functionality.
27-31: LGTM! Correct field annotations.The field declarations properly use Testsigma SDK annotations for test data input and test case result access.
36-44: LGTM! Proper BiDi Network setup.The Network instance creation using the WebDriver is correct according to Selenium BiDi API, and the target URL extraction is handled properly.
89-96: LGTM! Proper error handling and result reporting.The main exception handling correctly catches setup errors and returns appropriate results. The success condition (listener setup) vs. the actual interception is properly handled.
73-79: LGTM! Proper data structure for integration.The JSON structure created here correctly matches the expected format in
ResponseDataUtilities.getSpecificHeaderValue(), ensuring proper integration with theGetDataFromRequestBodyaction.
| @Data | ||
| @Action(actionText = "bidi : get value of the request header from the attribute header_key " + | ||
| "and store the value in runtime variable variable_name", | ||
| description = "Extracts header value from network request and stores it in runtime variable, please make " + | ||
| "sure that the request type is POST or PUT.", | ||
| applicationType = ApplicationType.WEB) | ||
| public class GetDataFromRequestBody extends WebAction { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Class name is misleading and doesn't reflect actual functionality.
The class name "GetDataFromRequestBody" suggests it extracts data from request body, but it actually extracts header values from network requests. Consider renaming to something more accurate like "GetHeaderValueFromNetworkRequest".
The action description also mentions "request type is POST or PUT" but there's no validation of HTTP method in the implementation.
🤖 Prompt for AI Agents
In
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/GetDataFromRequestBody.java
around lines 15 to 21, rename the class from GetDataFromRequestBody to a name
that reflects its functionality, such as GetHeaderValueFromNetworkRequest, to
match its purpose of extracting header values. Additionally, add validation
logic to check that the HTTP request method is POST or PUT as mentioned in the
action description, and handle cases where the method does not match these
types.
|
|
||
| public class FileUtilities { | ||
|
|
||
| private static final String folderPath = "/tmp/ntuc_data"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hardcoded file path reduces flexibility and portability.
The hardcoded path "/tmp/ntuc_data" may not work across different operating systems and environments. Consider making this configurable through system properties or environment variables.
- private static final String folderPath = "/tmp/ntuc_data";
+ private static final String folderPath = System.getProperty("testsigma.data.path", "/tmp/ntuc_data");📝 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 static final String folderPath = "/tmp/ntuc_data"; | |
| private static final String folderPath = System.getProperty("testsigma.data.path", "/tmp/ntuc_data"); |
🤖 Prompt for AI Agents
In
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/utilities/FileUtilities.java
at line 12, the folderPath is hardcoded to "/tmp/ntuc_data", which reduces
flexibility and portability. Modify the code to read this path from a system
property or environment variable instead, providing a default value if none is
set. This will allow the path to be configured dynamically based on the
deployment environment.
| FileUtils.writeStringToFile(file, content, StandardCharsets.UTF_8); | ||
| // Write the JSON content to the file | ||
| FileUtils.writeStringToFile(file, content, StandardCharsets.UTF_8); |
There was a problem hiding this comment.
Critical bug: Duplicate file write operations.
The content is being written to the file twice, which is unnecessary and could cause performance issues or unexpected behavior.
// Write the content to the file
FileUtils.writeStringToFile(file, content, StandardCharsets.UTF_8);
- // Write the JSON content to the file
- FileUtils.writeStringToFile(file, content, StandardCharsets.UTF_8);🤖 Prompt for AI Agents
In
store_network_logs_using_bidi/src/main/java/com/testsigma/addons/web/utilities/FileUtilities.java
between lines 48 and 50, there is a duplicate call to
FileUtils.writeStringToFile writing the same content twice to the file. Remove
the redundant second call to ensure the file is written only once, preventing
unnecessary performance overhead and potential issues.
| @@ -0,0 +1 @@ | |||
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyNDBkMjhiNS0xNmUwLThlNmYtOWQ0ZS05MjYxMGNiZTcyYzciLCJ1bmlxdWVJZCI6IjUxMzEiLCJpZGVudGl0eUFjY291bnRVVUlkIjoiMzUifQ.txkslZOqS4QYrEUTc6Xf9e_nbcYjkAX7N8zRo92AqNpncJoajYi4pY28iNI0Wvm6mr250-CSXYmAGavy-J2ICA No newline at end of file | |||
There was a problem hiding this comment.
Critical security issue: Hardcoded JWT token exposed in configuration file.
The JWT token is directly embedded in the properties file, which poses a significant security risk if this repository is public or shared. This token could provide unauthorized access to TestSigma services.
Move the API key to an environment variable or secure configuration system:
-testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyNDBkMjhiNS0xNmUwLThlNmYtOWQ0ZS05MjYxMGNiZTcyYzciLCJ1bmlxdWVJZCI6IjUxMzEiLCJpZGVudGl0eUFjY291bnRVVUlkIjoiMzUifQ.txkslZOqS4QYrEUTc6Xf9e_nbcYjkAX7N8zRo92AqNpncJoajYi4pY28iNI0Wvm6mr250-CSXYmAGavy-J2ICA
+testsigma-sdk.api.key=${TESTSIGMA_API_KEY}Alternatively, remove this file entirely and configure the API key through environment variables or a secure secrets management system.
📝 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.
| testsigma-sdk.api.key=eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIyNDBkMjhiNS0xNmUwLThlNmYtOWQ0ZS05MjYxMGNiZTcyYzciLCJ1bmlxdWVJZCI6IjUxMzEiLCJpZGVudGl0eUFjY291bnRVVUlkIjoiMzUifQ.txkslZOqS4QYrEUTc6Xf9e_nbcYjkAX7N8zRo92AqNpncJoajYi4pY28iNI0Wvm6mr250-CSXYmAGavy-J2ICA | |
| testsigma-sdk.api.key=${TESTSIGMA_API_KEY} |
🧰 Tools
🪛 Gitleaks (8.27.2)
1-1: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🤖 Prompt for AI Agents
In store_network_logs_using_bidi/src/main/resources/testsigma-sdk.properties at
line 1, the JWT token is hardcoded, posing a critical security risk. Remove the
hardcoded token from this file and instead configure the API key via environment
variables or a secure secrets management system. Ensure the application reads
the API key from these secure sources at runtime rather than from the properties
file.
#Jira
https://testsigma.atlassian.net/browse/TE-27807
Addon name : store network logs using bidi
Addon account : [email protected]
jarvis link : https://jarvis.testsigma.com/ui/tenants/2817/addons
fix
used selenium bidi to capture the request headers.
Summary by CodeRabbit
New Features
Chores