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

Huge exception message when dealing with large strings and column size restriction #1666

Closed
simboel opened this issue Jan 10, 2023 · 0 comments · Fixed by #1667
Closed

Huge exception message when dealing with large strings and column size restriction #1666

simboel opened this issue Jan 10, 2023 · 0 comments · Fixed by #1667

Comments

@simboel
Copy link
Contributor

simboel commented Jan 10, 2023

Note: See Pull Request #1667

Problem

When writing an entity to the database that exceeds the column limit (i.e. string with 200 characters in varchar(32)) the value will be printed in the message of the raised IllegalArgumentException in the format

throw IllegalArgumentException("Value '$value' can't be stored to database column because exceeds length ($colLength)")

For small strings this is no problem, but when dealing with huge binary data or strings (like base64 encoded data) the Exception message will grow out of hand quickly. Additionally this may expose sensitive data to the logs (passwords, GDPR-related data or other secrets).

I've encountered this problem when a JUnit test failed but was shown as still running in IDEA. After some investigation I've found that IDEA wasn't able to parse the logs to determine the current state of the test because the value I tried to write to the database was roughly 500,000 characters in size.

Proposed solution

I could think of multiple solutions:

  1. Exposed shouldn't mention the value at all (just state that the size of the value is greater than the column length and provide both numbers). Example: "Value can't be stored to database column because exceeds length ($valueLength > $length)"
  2. Exposed should only mention the first N characters/bytes of the value. Example: "Value '${value.take(50)}' can't be stored to database column because exceeds length ($colLength)". Maybe when more than N characters are present, Exposed should add [...] to the value representation.

Solution 2 would preserve the status quo without bloating the logs or output. But I'm unsure if the user really wants to know the value here. In theory this exception should never be thrown if the application code is correct. And if it is thrown, the user may only be interested in the length of the value (because he probably just made a calculation mistake for the column length). Together with the security/privacy reasoning I'm thinking the right choice would be solution 1 from above.

What are your thoughts about this? Pull request with solution 1: #1667

@simboel simboel changed the title Huge Huge exception message when dealing with large strings and column size restriction Jan 10, 2023
simboel added a commit to simboel/Exposed that referenced this issue Jan 10, 2023
Tapac pushed a commit that referenced this issue Jan 16, 2023
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

Successfully merging a pull request may close this issue.

1 participant