OpenOrienteering / mapper

OpenOrienteering Mapper is a software for creating maps for the orienteering sport.
https://www.openorienteering.org/apps/mapper/
GNU General Public License v3.0
404 stars 108 forks source link

OcdFileImport: Handle text objects without termination #2207

Closed dl3sdo closed 7 months ago

dl3sdo commented 8 months ago

As some .ocd files contain text objects without terminating null character limit the search for a terminating null character to the data area of the string. Closes #2206

dg0yt commented 8 months ago

I understood #2206 as indicating a problem with saving OCD files, not with loading OCD files. We do fill remain space with zero, but what if we didn't reserve extra space? https://github.com/OpenOrienteering/mapper/blob/d42dbf49ec4fea4fd9389426b57ac485614bd795/src/fileformats/ocd_file_export.cpp#L3117-L3121

dl3sdo commented 8 months ago

Hello Kai, you are absolutely right. The error is caused by an invalid export in Mapper which can easily be reproduced for every text object with a multiple of 32 characters. I apologize for not having read the issue description carefully enough where it was stated that the .ocd files had been created by Mapper. I will create a new PR (which should just contain an additional '+ 1').

lpechacek commented 7 months ago

@dl3sdo, while reading through the patch, I noticed that OcdFileImport::getObjectText() functionality is implemented twice. The OCD v8 version is correct while the generic version is buggy. I suggest that we reuse the v8 code for all the format versions.

Example implementation is at https://github.com/lpechacek/mapper/commits/issue-2206-garbage-in-text/.

dl3sdo commented 7 months ago

Libor, you are absolutely right, it makes little sense to have two implementations.

dg0yt commented 7 months ago

Closing in favor of https://github.com/OpenOrienteering/mapper/pull/2219.