raylib
raylib copied to clipboard
raysan5
[rtextures] fix buffer overflow on loading .svg file
simple fix for #4185
Makes sure the input to nsvgParse() function is NULL terminated, to avoid buffer overflow
Not OPs fault but this function has terrible API. fileNameOrString ?? There should be two separate functions.
Any reason to not just use
nsvgParseFromFile?
Given the existence of the custom callback function for loading files (LoadFileDataCallback), and that that nsvgParseFromFile wasn't used from the beginning
I presumed the idea was to centralize file I/O to the internal functions
So if the user wants to do something weird with I/O, or for some reason raylib I/O gets updated, it's easier to change
Not OPs fault but this function has terrible API.
fileNameOrString?? There should be two separate functions.
I could understand the idea behind having only one function for parsing both file and raw string of SVG, It wouldn't make things terribly more complicated, and is rather elegant
But given this thing is something more educational, this is probably a bad idea, obscures the operation
The functionality should be broken into two functions, to keep parity with LoadImage and LoadImageFromMemory
Thought i feel that that might be out of scope for this PR?
I just realized LoadImage is also able to load SVGs,
verified that it had the same problem and extended the fix for it.
The fact that the generic function can also load SVGs does explain why LoadImageSvg is the way it is,
it is just the alternative if you need a specific scale, thus, it is compacted to avoid clutter
@spartan-engi thanks for the review. About the specific function, note that raylib API has no file-format-specific function.
@spartan-engi I'm cosnidering removing SVG support all together...
Oh, no. I would ask to add support for processing animated SVGs, but there is still a mess with the implementation standards :D
I decided to remove SVG support from raylib and move the required code to a separate example, the LoadImageSvg() API is ugly (and the only format-dependant function in the raylib API); also, as this issue states, it could generate issues requiring ugly hacks. Also, it adds extra maintenance burden. And also, it's the only format that requries several external libraries to work. Also, the format is disabled by default so most bindings probably don't care about it.
For all those reasons I'm moving the functionality and the example raylib-extras. I really think there is no need to be in raylib.