6 Dec 2023 |
morg7672 | not sure if the issue has always been or if it is a regression | 09:02:02 |
morg7672 | This commit maybe https://gitlab.com/CalcProgrammer1/OpenRGB/-/commit/d7ed55b264dee40f68e7a17c11eaa8f1b56d8dc6 | 09:05:32 |
morg7672 | Yeah that was it | 09:09:26 |
morg7672 | We have to remove the trailing 0, not sure where to fix that. Either before calling
profile_manager->LoadProfile(profile_name);
Or inside ProfileManager::LoadProfile, or both..?
The fix :
while (!profile_name.empty() && profile_name.back() == 0)
{
profile_name.pop_back();
} | 09:10:43 |
morg7672 | https://gitlab.com/CalcProgrammer1/OpenRGB/-/merge_requests/2180 ryleu you can try this build | 09:38:26 |
chr1sno | If this is an issue in the network manager why are we applying the fix in the profile manager?? | 09:41:36 |
morg7672 | As it's a public method, I think the check has to be done here to insure the input validity. It will also benefits to all other calls to LoadProfile/SaveProfile/DeleteProfile. | 09:48:54 |
morg7672 | The issue is in ProfileManager, not in NetworkManager | 09:49:28 |
chr1sno | The problem is coming from the network manager specifically not handling strings properly | 09:49:39 |
morg7672 | that's where the strings are concatenated | 09:49:39 |
chr1sno | If that were the case we would have issues with every other "load profile" call which we don't. What is the value of the string in the network manager prior to calling LoadProfile()?? | 09:50:59 |
morg7672 | same as shown in the screenshot of the debug panel | 09:51:24 |
morg7672 | ^ | 09:51:29 |
chr1sno | So the network manager is passing around a bad string | 09:51:56 |
chr1sno | that's were the problem should be fixed. | 09:52:07 |
chr1sno | What happens if you change line 800 to
profile_name.assign(data);
and let the assign function find the null character? | 09:56:17 |
morg7672 | according to the assign reference, it will copy the null char as well | 09:59:11 |
morg7672 | c-string (3)
string& assign (const char* s);
(3) c-string
Copies the null-terminated character sequence (C-string) pointed by s. | 09:59:28 |
morg7672 | or maybe i'm not understanding it correctly? | 10:00:19 |
morg7672 | yeah, looks like it removes the null char | 10:01:42 |
chr1sno | I think passing the size is forcing the assign function to copy the null character into the data part of the string. | 10:04:58 |
chr1sno | 🤔 Concerning that no data inbound is being sanitised. | 10:07:18 |
ryleu | i can try that build after i get back | 23:13:05 |
7 Dec 2023 |
| ne0_951 changed their display name from NE0_93 to ne0_951. | 01:35:54 |
| satired_ changed their display name from Satired to satired_. | 02:24:17 |
| deat0101010 changed their profile picture. | 18:55:29 |
| zeerbeste changed their display name from thomas to zeerbeste. | 22:33:36 |
9 Dec 2023 |
| terdog changed their profile picture. | 23:09:10 |
13 Dec 2023 |
| frostybiscuit changed their display name from FrostyBiscuit to frostybiscuit. | 16:44:46 |
| campblor changed their display name from Campblor to campblor. | 20:38:05 |