Skip to content
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

Sanitize manufacturer name to avoid json errors. #1235

Merged

Conversation

ranma
Copy link

@ranma ranma commented Sep 6, 2024

If the string contains control characters for some reason, the browser will reject the json with the error bad control character in string literal.

See #1226 (comment)

@ranma ranma force-pushed the pylontech-sanitize-manufacturer branch from 9393030 to 5a2ad78 Compare September 7, 2024 06:52
src/BatteryStats.cpp Outdated Show resolved Hide resolved
@ranma ranma force-pushed the pylontech-sanitize-manufacturer branch from 5a2ad78 to 837c1c6 Compare September 7, 2024 11:25
src/PylontechCanReceiver.cpp Outdated Show resolved Hide resolved
@ranma ranma force-pushed the pylontech-sanitize-manufacturer branch from 837c1c6 to 1b4aa4c Compare September 8, 2024 16:41
@ranma
Copy link
Author

ranma commented Sep 9, 2024

maybe it would make even more sense to add string validation to addLiveViewInSection(), rather than specifically as a workaround for bad manufacturer strings.

@AndreasBoehm
Copy link
Member

imho it makes sense to do the cleanup when we set the manufacturer string because it always makes sense to fix issues at the source of the problem and we should not pass around bad strings

@schlimmchen
Copy link
Member

I agree with Andreas. The new function is not indented properly. I am going to fix this now so this can be merged.

If the string contains control characters for some reason, the browser
will reject the json with the error `bad control character in string
literal`.

This adds a setManufacturer function that validates the string is ASCII
and will cut off the string at the first non-ascii character.

Pylontech: `PYLON` (50 59 4C 4F 4E 20 20 20)
Pytes: `PYTES` (50 59 54 45 53)
Deye: `DY001` (44 59 30 30 31 03 E8 03)

See hoylabs#1226 (comment)
@schlimmchen schlimmchen force-pushed the pylontech-sanitize-manufacturer branch from 1b4aa4c to 1af5a2c Compare September 9, 2024 19:41
@schlimmchen schlimmchen merged commit f42f018 into hoylabs:development Sep 9, 2024
8 checks passed
@schlimmchen
Copy link
Member

Thanks, @ranma 🚀

Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2024
@ranma ranma deleted the pylontech-sanitize-manufacturer branch October 10, 2024 15:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants