Catch exception when assiging String<> type with too long string #966
Conversation
marcemmers
requested review from
Pietfried,
hikinggrass and
maaikez
as code owners
|
I think we can still end up having issues here with the SecurityEventNotification with |
|
To be honest I did not check further, this was just an issue I found with some incoming message tests that we did. I am not sure what you are referring to here? Isn't that CiString created by the application for the SecurityEventNotification? In that case there will be an error thrown that the application can/should catch right? |
lib/ocpp/v201/charge_point.cpp
Outdated
| this->message_dispatcher->dispatch_call_error( | ||
| CallError(MessageId("-1"), "RpcFrameworkError", e.what(), json({}))); | ||
| const auto& security_event = ocpp::security_events::INVALIDMESSAGES; | ||
| this->security->security_event_notification_req(CiString<50>(security_event), CiString<255>(message), true, |
There was a problem hiding this comment.
This can still throw if message > 255 characters or security_event > 50 characters.
It might be an option to truncate CiStrings by default inside the class in case the number of characters exceeds the specified limit instead of throwing the exception. I dont have a complete overview but I think it can happen at quite a few places
There was a problem hiding this comment.
I've added an argument that can optionally truncate if we want. I don't think truncating by default is an option because that might lead to strange behavior as well. We would silently accept incorrect messages from the BO for example.
include/ocpp/common/string.hpp
Outdated
| using std::runtime_error::runtime_error; | ||
| }; | ||
|
|
||
| enum class StringToLarge { |
include/ocpp/common/string.hpp
Outdated
|
|
||
| public: | ||
| /// \brief Creates a string from the given \p data | ||
| String(const std::string& data) : length(L) { | ||
| this->set(data); | ||
| String(const std::string& data, StringToLarge to_large = StringToLarge::Throw) { |
There was a problem hiding this comment.
| String(const std::string& data, StringToLarge to_large = StringToLarge::Throw) { | |
| explicit String(const std::string& data, StringToLarge to_large = StringToLarge::Throw) { |
include/ocpp/common/string.hpp
Outdated
| } | ||
|
|
||
| String(const char* data) : length(L) { | ||
| this->set(data); | ||
| String(const char* data, StringToLarge to_large = StringToLarge::Throw) { |
There was a problem hiding this comment.
| String(const char* data, StringToLarge to_large = StringToLarge::Throw) { | |
| explicit String(const char* data, StringToLarge to_large = StringToLarge::Throw) { |
Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>
Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com>
…ariable which can be done purely compile time Signed-off-by: Marc Emmers <m.emmers@alfen.com>
Signed-off-by: Piet Gömpel <pietgoempel@gmail.com>
Pietfried
force-pushed
the
me-fix-string-to-long-crash
branch
from
3060fa5 to
47351c5
Compare
* Catch exception when assiging String<> type with too long string * Add choice between throw and truncating, also remove runtime length variable which can be done purely compile time --------- Signed-off-by: Marc Emmers <m.emmers@alfen.com> Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com> Signed-off-by: Piet Gömpel <pietgoempel@gmail.com> Co-authored-by: Piet Gömpel <pietgoempel@gmail.com>
* Catch exception when assiging String<> type with too long string * Add choice between throw and truncating, also remove runtime length variable which can be done purely compile time --------- Signed-off-by: Marc Emmers <m.emmers@alfen.com> Signed-off-by: Marc Emmers <35759328+marcemmers@users.noreply.github.com> Signed-off-by: Piet Gömpel <pietgoempel@gmail.com> Co-authored-by: Piet Gömpel <pietgoempel@gmail.com> Signed-off-by: Maaike Zijderveld, iolar <git.mail@iolar.nl>
Describe your changes
Fixes a crash when the MessageId string received is longer than 36 characters. Instead of crashing we now send an RpcFrameworkError.
Issue ticket number and link
Checklist before requesting a review