Skip to content

Commit

Permalink
Fix: sanitize battery manufacturer name string (#1235)
Browse files Browse the repository at this point in the history
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 #1226 (comment)
  • Loading branch information
ranma authored Sep 9, 2024
1 parent cec4003 commit f42f018
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 9 deletions.
6 changes: 3 additions & 3 deletions include/BatteryStats.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,15 @@ class BatteryStats {
_lastUpdateCurrent = _lastUpdate = timestamp;
}

String _manufacturer = "unknown";
void setManufacturer(const String& m);

String _hwversion = "";
String _fwversion = "";
String _serial = "";
uint32_t _lastUpdate = 0;

private:
String _manufacturer = "unknown";
uint32_t _lastMqttPublish = 0;
float _soc = 0;
uint8_t _socPrecision = 0; // decimal places
Expand All @@ -98,7 +100,6 @@ class PylontechBatteryStats : public BatteryStats {
float getChargeCurrentLimitation() const { return _chargeCurrentLimitation; } ;

private:
void setManufacturer(String&& m) { _manufacturer = std::move(m); }
void setLastUpdate(uint32_t ts) { _lastUpdate = ts; }

float _chargeVoltage;
Expand Down Expand Up @@ -137,7 +138,6 @@ class PytesBatteryStats : public BatteryStats {
float getChargeCurrentLimitation() const { return _chargeCurrentLimit; } ;

private:
void setManufacturer(String&& m) { _manufacturer = std::move(m); }
void setLastUpdate(uint32_t ts) { _lastUpdate = ts; }
void updateSerial() {
if (!_serialPart1.isEmpty() && !_serialPart2.isEmpty()) {
Expand Down
21 changes: 17 additions & 4 deletions src/BatteryStats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,19 @@ static void addLiveViewAlarm(JsonVariant& root, std::string const& name,
root["issues"][name] = 2;
}

void BatteryStats::setManufacturer(const String& m)
{
String sanitized(m);
for (int i = 0; i < sanitized.length(); i++) {
char c = sanitized[i];
if (c < 0x20 || c >= 0x80) {
sanitized.remove(i); // Truncate string
break;
}
}
_manufacturer = std::move(sanitized);
}

bool BatteryStats::updateAvailable(uint32_t since) const
{
if (_lastUpdate == 0) { return false; } // no data at all processed yet
Expand Down Expand Up @@ -475,18 +488,18 @@ void JkBmsBatteryStats::updateFrom(JkBms::DataPointContainer const& dp)
{
using Label = JkBms::DataPointLabel;

_manufacturer = "JKBMS";
setManufacturer("JKBMS");
auto oProductId = dp.get<Label::ProductId>();
if (oProductId.has_value()) {
// the first twelve chars are expected to be the "User Private Data"
// setting (see smartphone app). the remainder is expected be the BMS
// name, which can be changed at will using the smartphone app. so
// there is not always a "JK" in this string. if there is, we still cut
// the string there to avoid possible regressions.
_manufacturer = oProductId->substr(12).c_str();
setManufacturer(String(oProductId->substr(12).c_str()));
auto pos = oProductId->rfind("JK");
if (pos != std::string::npos) {
_manufacturer = oProductId->substr(pos).c_str();
setManufacturer(String(oProductId->substr(pos).c_str()));
}
}

Expand Down Expand Up @@ -558,7 +571,7 @@ void VictronSmartShuntStats::updateFrom(VeDirectShuntController::data_t const& s
_timeToGo = shuntData.TTG / 60;
_chargedEnergy = static_cast<float>(shuntData.H18) / 100;
_dischargedEnergy = static_cast<float>(shuntData.H17) / 100;
_manufacturer = String("Victron ") + shuntData.getPidAsString().data();
setManufacturer(String("Victron ") + shuntData.getPidAsString().data());
_temperature = shuntData.T;
_tempPresent = shuntData.tempPresent;
_midpointVoltage = static_cast<float>(shuntData.VM) / 1000;
Expand Down
2 changes: 1 addition & 1 deletion src/PylontechCanReceiver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ void PylontechCanReceiver::onMessage(twai_message_t rx_message)
MessageOutput.printf("[Pylontech] Manufacturer: %s\r\n", manufacturer.c_str());
}

_stats->setManufacturer(std::move(manufacturer));
_stats->setManufacturer(manufacturer);
break;
}

Expand Down
2 changes: 1 addition & 1 deletion src/PytesCanReceiver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ void PytesCanReceiver::onMessage(twai_message_t rx_message)
MessageOutput.printf("[Pytes] Manufacturer: %s\r\n", manufacturer.c_str());
}

_stats->setManufacturer(std::move(manufacturer));
_stats->setManufacturer(manufacturer);
break;
}

Expand Down

0 comments on commit f42f018

Please sign in to comment.