Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[text] TextFormat() silently fails when the formatted text is longer than MAX_TEXT_BUFFER_LENGTH #3366

Closed
danielchasehooper opened this issue Oct 2, 2023 · 10 comments

Comments

@danielchasehooper
Copy link

Issue description

TextFormat() silently truncates when the formatted text is longer than MAX_TEXT_BUFFER_LENGTH. I spent a long time thinking there was an issue elsewhere but TextFormat was just truncating the result and not aborting or logging a message.

Environment

macOS

Code Example

char *someReallyLongString; // must be longer than MAX_TEXT_BUFFER_LENGTH
char *result = TextFormat("File contents:\n%s", someReallyLongString);
@ghost
Copy link

ghost commented Oct 2, 2023

@danielchasehooper I could be wrong, but I believe data validation is up to the user on raylib (reference #3246 (comment)).

@danielchasehooper
Copy link
Author

It's not a data validation issue. the parameters sending to the function were valid. The function doesn't do what it says it will do for certain inputs, and doesn't warn you when thats happening.

@ghost
Copy link

ghost commented Oct 2, 2023

I don't disagree, but, IMHO, you still have to control what you're passing to the functions. If you look at other the library functions, you'll see very few of them have complete/full parameter validation.

@raysan5
Copy link
Owner

raysan5 commented Oct 2, 2023

I could be wrong, but I believe data validation is up to the user on raylib

@ubkp Yes, you are right, raylib does not consider all the possible (wrong?) use case scenarios, it would imply lots of functions review and increase in code complexity.

That was the original approach of the library that actually fit very well on the original education context to warn the students about writing valid code.

Still, in the past years many checks and validations have been added to the library, specially for memory-critic scenarios. I think in this specific case it could log some warning... but for this specific function the warning will be logged 60 times per second, that's something that I also avoid in raylib.

An easy solution I can think of is appending a "TRUNCATED" (or similar) to the input string, so the users will be aware that something happened in there.

@ghost
Copy link

ghost commented Oct 2, 2023

An easy solution I can think of is appending a "TRUNCATED" (or similar) to the input string, so the users will be aware that something happened in there.

@raysan5 This would be an excellent solution.

@danielchasehooper
Copy link
Author

I like that idea

@raysan5 raysan5 changed the title TextFormat silently fails when the formatted text is longer than MAX_TEXT_BUFFER_LENGTH [text] TextFormat() silently fails when the formatted text is longer than MAX_TEXT_BUFFER_LENGTH Oct 3, 2023
@Murlocohol
Copy link
Contributor

So I looked into this and implementing it is rather trivial because vsnprintf() is used. vsnprintf() returns the number of chars that would have been required to make the string, so all we need to do is just compare that to MAX_TEXT_BUFFER_LENGTH and then we will know if we have overflow or not.

I just posted a PR with this fix (I tried to make sure there wasn't any weirdness this time): #3399

Using the following code:

TraceLog(LOG_INFO, TextFormat("The quick brown fox jumps over the lazy dog."));

Before the fix with a 1024 buffer generates:

\-----------------------------
INFO: The quick brown fox jumps over the lazy dog.
[Finished in 313ms]

Now if I change the MAX_TEXT_BUFFER_LENGTH to 16:

\-----------------------------
INFO: The quick brown
[Finished in 292ms]

Now using the suggested fix:

\-----------------------------
WARNING: RTEXT: TextFormat string was [TRUN]cated. If you need longer strings, please increase MAX_TEXT_BUFFER_LENGTH.
INFO: The quick[TRUN]
[Finished in 297ms]

@orcmid
Copy link
Contributor

orcmid commented Oct 11, 2023

That was quick. I suggest,

  1. Using "..." instead of the TRUN Englishism. Or else,
  2. In accord with the perfect programmer presumption, silently truncating is the consistent action, with a comment in the code that the function will do that.

raysan5 added a commit that referenced this issue Oct 11, 2023
It seems more standard than [TRUN]
@raysan5
Copy link
Owner

raysan5 commented Oct 11, 2023

@orcmid I agree with your concern, just modified it.

@orcmid
Copy link
Contributor

orcmid commented Oct 12, 2023

Possible Pitfall: If TextFormat() accepts and possibly incorporates muti-byte (especially UTF8) encodings, backing up 4 characters is not necessarily the same as backing up 4 bytes. It is then necessary to do this in a way that does not create an invalid multi-byte or UTF8 string encoding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants