----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46475/#review130733 -----------------------------------------------------------
Ship it! Ship It! - Andrii Tkach On April 26, 2016, 9:09 p.m., Keta Patel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46475/ > ----------------------------------------------------------- > > (Updated April 26, 2016, 9:09 p.m.) > > > Review request for Ambari, Andrii Tkach and Di Li. > > > Bugs: AMBARI-15979 > https://issues.apache.org/jira/browse/AMBARI-15979 > > > Repository: ambari > > > Description > ------- > > ISSUE: > The UI validation at present checks only the length of the user input for > widget_name and description fields. All characters are allowed to be stored > in the database through them. A more strict UI validation that limits the > type of characters entered for these fields will provide a good first line of > defense. > > Steps to reproduce: > 1. Make sure you have Ambari Metrics service installed on your cluster. > 2. On the Dashboard, select any service that makes use of Ambari Metrics, say > HDFS. > 3. In the "Metrics" section, click the "Actions" button in the top-right > corner, and select "Create a new widget" option from the drop-down. > (attachment: create_widget_button_location.tiff) > 4. Create widget pop-up is displayed. > 5. On Step-1, select any type for the widget and click "Next". (attachment: > create_widget_step1.tiff) > 6. On Step-2, select any valid metrics parameter and click "Next". > (attachment: create_widget_step2.tiff) > 7. On Step-3, for widget_name and description fields, you can enter any > character. No validation is present to check the contents. The only > validation present checks the length of the input text. > (attachments: > create_widget_step3.tiff, > original_characters_allowed_for_name_and_description.tiff, > original_length_validation_for_name.tiff, > original_length_validation_for_description.tiff ) > > > FIX: > The UI validation is enhanced by checking the content of the input for name > and description. > The patch attached allows only alphanumeric, underscore, hyphen, space and > percentage symbol to be valid characters for both fields. > The % symbol is added as part of the white-list as there are existing widgets > that contain % symbols in their names. In order to keep the characters in the > name consistent, this was decided. > The description could probably have a little more flexibility in terms of > characters allowed. Any suggestions to update this validation would be > helpful. > The warning message template is also updated to conform to the existing norms > used in several Ambari pop-ups (e.g. Manage Config Groups, Manage Alert > Groups). > > The images attached (starting with "fixed_") show how the fix appears with > the patch. > > > Diffs > ----- > > ambari-web/app/controllers/main/service/widgets/create/step3_controller.js > dd7a93f > ambari-web/app/messages.js 8c8b9e5 > ambari-web/app/templates/main/service/widgets/create/step3.hbs 9f431af > ambari-web/app/utils/validator.js 490fec5 > > ambari-web/test/controllers/main/service/widgets/create/step3_controller_test.js > 6f92142 > > Diff: https://reviews.apache.org/r/46475/diff/ > > > Testing > ------- > > Tests are added that check the validate functions added for widget name and > description. > Both widget name and description are tested for: > 1. all valid characters > 2. invalid characters > 3. length of input > 4. empty string > > Ambari-Web tests with the patch: > 25671 tests complete (33 seconds) > 154 tests pending > > > File Attachments > ---------------- > > AMBARI-15979-inlineError.patch > > https://reviews.apache.org/media/uploaded/files/2016/04/21/03bc972a-b076-4520-948b-3a204082eca0__AMBARI-15979-inlineError.patch > AMBARI-15979-topError.patch > > https://reviews.apache.org/media/uploaded/files/2016/04/21/8b4e9a7b-96cb-4a17-9c9c-56e59b5fd349__AMBARI-15979-topError.patch > AMBARI-15979-April-26-updated.patch > > https://reviews.apache.org/media/uploaded/files/2016/04/26/8631c255-73b9-4179-ba70-6372a69ac225__AMBARI-15979-April-26-updated.patch > > > Thanks, > > Keta Patel > >