Add ability to download transaction meter values as CSV#2044
Merged
Conversation
Review Summary by QodoAdd transaction meter values CSV download functionality
WalkthroughsDescription• Add CSV download capability for transaction meter values • Implement CsvMapper with Jackson dataformat-csv dependency • Define MeterValues field ordering via @JsonPropertyOrder annotation • Create new endpoint for downloading meter values as CSV file Diagramflowchart LR
A["TransactionDetails.MeterValues"] -->|"@JsonPropertyOrder"| B["Field Ordering"]
C["CsvMapper"] -->|"schema configuration"| D["CSV Serialization"]
E["New Endpoint"] -->|"GET /transactions/details/{id}/meterValues.csv"| F["CSV Download"]
D -->|"writes"| F
B -->|"used by"| D
File Changes1. src/main/java/de/rwth/idsg/steve/repository/dto/TransactionDetails.java
|
Code Review by Qodo
1. Meter CSV leaks exception details
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+98
to
+107
| @RequestMapping(value = TRANSACTIONS_DETAILS_METER_VALUES_CSV_PATH) | ||
| public void getTransactionDetailsMeterValuesCsv(@PathVariable("transactionPk") int transactionPk, | ||
| HttpServletResponse response) throws IOException { | ||
| String fileName = "transaction_%s_meter_values.csv".formatted(transactionPk); | ||
| String headerKey = "Content-Disposition"; | ||
| String headerValue = "attachment; filename=\"%s\"".formatted(fileName); | ||
| response.setContentType("text/csv"); | ||
| response.setHeader(headerKey, headerValue); | ||
| transactionService.writeTransactionMeterValuesCSV(transactionPk, response.getWriter()); | ||
| } |
There was a problem hiding this comment.
1. Meter csv leaks exception details 📘 Rule violation ⛨ Security
The new CSV download endpoint does not handle not-found/serialization failures and will fall back to
the global MVC error page, which renders ${exception}/${exception.cause} to the client
(information leakage) instead of a client-safe error response.
Agent Prompt
## Issue description
The new `meterValues.csv` endpoint does not catch/translate expected errors (e.g., transaction not found) into client-safe responses, and the MVC global error view displays exception details to the user.
## Issue Context
For download endpoints, not-found should map to a 404-style response and error messages must be client-safe (no internal exception details).
## Fix Focus Areas
- src/main/java/de/rwth/idsg/steve/web/controller/TransactionsReservationsController.java[98-107]
- src/main/java/de/rwth/idsg/steve/web/GlobalControllerAdvice.java[62-69]
- src/main/webapp/WEB-INF/views/00-error.jsp[22-28]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+74
to
+77
| public void writeTransactionMeterValuesCSV(int transactionPk, Writer writer) { | ||
| try (var seqWriter = csvMapper.writer(schema).writeValues(writer)) { | ||
| seqWriter.writeAll(getDetails(transactionPk).getValues()); | ||
| } |
There was a problem hiding this comment.
2. Csv formula injection 🐞 Bug ⛨ Security
writeTransactionMeterValuesCSV exports raw string fields into a downloadable CSV without neutralizing spreadsheet-formula prefixes, so values starting with '=', '+', '-' or '@' can execute as formulas when opened in Excel/Sheets. This enables CSV injection from device/user-controlled meter-value fields.
Agent Prompt
### Issue description
The meter-values CSV export writes untrusted strings directly to CSV. Spreadsheet apps may interpret fields starting with `=`, `+`, `-`, or `@` as formulas, enabling CSV injection when users open the downloaded file.
### Issue Context
Meter-value fields (e.g., `readingContext`, `format`, `measurand`, `location`, `unit`, `phase`, and potentially `value`) originate from DB columns and are rendered as untrusted in the JSP via HTML encoding, but the CSV export path performs no equivalent neutralization.
### Fix
Before writing rows, sanitize all string fields that may be opened in spreadsheets:
- For any cell value whose first non-empty character is one of `= + - @`, prefix with a single quote `'` (or another agreed neutralization like leading tab) so Excel/Sheets treat it as literal text.
- Apply this to every exported string column, ideally via a small helper (`sanitizeForSpreadsheet(String)`), and map `MeterValues` into a sanitized DTO/copy prior to `seqWriter.writeAll(...)`.
### Fix Focus Areas
- src/main/java/de/rwth/idsg/steve/service/TransactionService.java[74-77]
- src/main/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImpl.java[230-245]
- src/main/webapp/WEB-INF/views/data-man/transactionDetails.jsp[64-74]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.