Skip to content

Commit

Permalink
Fix: sanitize battery manufacturer name string
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 and schlimmchen committed Sep 9, 2024
1 parent cec4003 commit 1af5a2c
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 1af5a2c

Please sign in to comment.