-
Notifications
You must be signed in to change notification settings - Fork 694
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
GUACAMOLE-1948: Provide a comprehensive error message for input exceeding database column. #983
GUACAMOLE-1948: Provide a comprehensive error message for input exceeding database column. #983
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good to me, but should the scope of this issue be expanded to include other things? For example, the Connection Name is limited to 128 characters - what happens (currently) if someone tries to enter a name longer than that?
Also, is there anything we can/should do on the fields themselves to check for this, in addition to (but maybe before) the Java code does its check? So, if we put a maximum length parameter on the fields, JavaScript + HTML could do the initial checks and limitations, with the DB verifying that nothing adds up to a value longer than is supported?
Maybe that's over-complicating things, so I'm fine if the answer is "no," but just thinking out loud...
Thanks for the feedback! I agree that we should ensure we handle limits like the one for connection names and parameters properly. For now, I targeted the most pressing issue, which was unclear errors when saving connection parameters that were long enough to cause database truncation errors. Implementing HTML field limits sounds simple at first, but our guacamole client's setup complicates it. We generate the HTML for the connection settings from JSON translation files for each language, like For the time being, the current fix should be sufficient in making the error messages better. We can look into adding client-side checks as a separate update later on. |
This looks reasonable to me, if you're ok with it @necouchman. |
I'm not sure I understand the issue, here, but we'll go with it. Solving the current issue can be the priority... |
@aleitner If this is a fix, should it go into |
…ding database column.
c1999d6
to
7317edd
Compare
Problem
We identified an issue where users were facing a generic error when attempting to save Guacamole connection parameters that exceeded the maximum allowed length (4096 characters). The error message was not clear and did not provide useful feedback regarding the input size constraint on the database column for connection parameters.
Solution
To enhance the user experience and provide a more informative error message, I have implemented the following changes:
Added a new method
validateParameters
within theConnectionService
class. This method checks each connection parameter's value against the known maximum size limit before a connection is created or updated. If a parameter value exceeds the maximum allowed length, the method throws aTranslatableGuacamoleClientOverrunException
with a clear and specific message.Added new error key
CONNECTION_PARAMETERS.DATABASE_PARAMETER_VALUE_TOO_LONG
to the JSON file of translatable messages.