-
Notifications
You must be signed in to change notification settings - Fork 10
fix(api): decode html entities before parsing notifications #1768
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
Conversation
so the parser does not treat them as comments
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe changes introduce HTML entity decoding to the notification file parsing pipeline. A new dependency, html-entities, is added to support decoding HTML-encoded content before INI parsing. The notification service is refactored to read file contents directly via fs/promises, decode HTML entities, and pass decoded content to the parser. Tests are updated to mock the readFile operation. Version incremented to 4.25.3. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Code
participant Service as Notification Service
participant FS as fs/promises
participant Decode as html-entities
participant Parser as INI Parser
rect rgb(220, 240, 220)
Note over Test,Parser: New Flow with HTML Decoding
Test->>Service: loadNotificationFile(filePath)
Service->>FS: readFile(filePath)
FS-->>Service: rawContent (HTML-encoded)
Service->>Decode: decode(rawContent)
Decode-->>Service: decodedContent
Service->>Parser: parse({file: decodedContent, type: 'ini'})
Parser-->>Service: notificationObject
Service-->>Test: notificationObject
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
🧰 Additional context used📓 Path-based instructions (7)api/src/unraid-api/**📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Files:
api/**/*.{test,spec}.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Files:
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
api/**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
{api,web}/**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
api/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (17)📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-09-12T01:36:59.838ZApplied to files:
📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-09-12T01:36:59.838ZApplied to files:
📚 Learning: 2025-09-12T01:36:59.838ZApplied to files:
📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-08-11T15:07:39.222ZApplied to files:
📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-08-11T15:10:28.150ZApplied to files:
📚 Learning: 2025-01-31T22:01:22.708ZApplied to files:
📚 Learning: 2025-04-21T18:44:39.643ZApplied to files:
🧬 Code graph analysis (1)api/src/unraid-api/graph/resolvers/notifications/notifications.service.ts (2)
🔇 Additional comments (14)
Comment |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| private async loadNotificationFile(path: string, type: NotificationType): Promise<Notification> { | ||
| const rawContent = await readFile(path, 'utf-8'); | ||
| const decodedContent = decode(rawContent); | ||
|
|
||
| const notificationFile = parseConfig<NotificationIni>({ | ||
| filePath: path, | ||
| file: decodedContent, | ||
| type: 'ini', |
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.
Hash characters in notification values are still stripped
Decoding the file before parseConfig removes HTML entities but the INI parser still treats # as a comment delimiter. If a notification subject or description legitimately contains a hash (for example the new Hashtag Test sample or real alerts like #1 disk is hot or hashtags), ini.parse will drop everything after the first #, so clients receive truncated text. Consider escaping hashes or configuring the parser so literal # characters survive.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1768 +/- ##
=======================================
Coverage 52.69% 52.69%
=======================================
Files 865 865
Lines 49343 49346 +3
Branches 4952 4952
=======================================
+ Hits 26000 26003 +3
Misses 23270 23270
Partials 73 73 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.26.0](v4.25.3...v4.26.0) (2025-11-17) ### Features * add cpu power query & subscription ([#1745](#1745)) ([d7aca81](d7aca81)) * add schema publishing to apollo studio ([#1772](#1772)) ([7e13202](7e13202)) * add workflow_dispatch trigger to schema publishing workflow ([818e7ce](818e7ce)) * apollo studio readme link ([c4cd0c6](c4cd0c6)) * **cli:** make `unraid-api plugins remove` scriptable ([#1774](#1774)) ([64eb9ce](64eb9ce)) * use persisted theme css to fix flashes on header ([#1784](#1784)) ([854b403](854b403)) ### Bug Fixes * **api:** decode html entities before parsing notifications ([#1768](#1768)) ([42406e7](42406e7)) * **connect:** disable api plugin if unraid plugin is absent ([#1773](#1773)) ([c264a18](c264a18)) * detection of flash backup activation state ([#1769](#1769)) ([d18eaf2](d18eaf2)) * re-add missing header gradient styles ([#1787](#1787)) ([f8a6785](f8a6785)) * respect OS safe mode in plugin loader ([#1775](#1775)) ([92af3b6](92af3b6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
so the parser does not treat them as comments.
This surfaces a new bug:
#'s in notification subject or descriptions are treated as comments, and content following a#will not be displayed in queries from the api, unless the values are explicitly quoted as strings:Summary by CodeRabbit
Version 4.25.3
Improvements
Chores