Maxlength validation counts newlines twice

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

For the tests requested in #4

I verified the problem on my Windows laptop. This is still an issue with the latest 11.x-dev.

The issue summary has been updated with information about the actual problem. This isn't the result of PHP "counting new line characters twice." The problem is that in Windows new lines are literally two characters, a carriage return and line feed (\r\n). JavaScript and browsers ignore the \r when calculating string length. PHP counts them both.

Thus, the former proposed fix and patch in #3 are unacceptable solutions. They would cause strings to pass form validation, but would eventually result Drupal trying to save strings that are too long into database fields whose sizes are set to the given maxlengths. That would result in even worse errors, probably WSoDs.

I don't know what the best solution is to the problem. A framework manager will have to decide.

I wrote a very basic FunctionalJavascript test that demonstrates the issue. The test fails because it finds the server-side validation error message after submitting the form. It will have to be modified appropriately after a solution for the bug has been decided, but it demonstrates that there's a problem for now. I wanted to make it verify the contents of the textarea field before submitting the form, but I don't know how. This is literally the first FunctionalJavascript test that I've written.

Anyway, I'm leaving the Needs Tests tag since it will have to be improved or modified. But I'm changing the status to Needs Review to get the attention of a maintainer for an opinion on a fix.

Form validation issues are described as having Major status.

Not 100% sure putting into review will grab the attention.

Though it's good to have the hard part done in a test.

Will post in slack though to try and kick start it.

Looking at the HTML5 Standard, the maxlength attribute for a form control is validated by its "API Value".

In the case of textarea elements, the API value and value differ. In particular, newline normalization is applied before the maximum allowed value length is checked (whereas the textarea wrapping transformation is not applied).

And the newline normalization converts "\r\n" and "\r" to "\n". So in PHP, the maxlength should be validated against the submitted value with the same newline normalization.

This likely affects submissions regardless of the user's brower/OS. Also per the HTML5 standard, the submitted form values should have "\n" and "\r" normalized to "\r\n". So the form values in PHP should have "\r\n" exclusively. Testing on Chrome in MacOS, I can validate that this is what I've observed. Putting a breakpoint on these lines in Drupal\Core\Form\FormValidator::performRequiredValidation():

    // Verify that the value is not longer than #maxlength.
    if (isset($elements['#maxlength']) && mb_strlen($elements['#value']) > $elements['#maxlength']) {
      $form_state->setError($elements, $this->t('@name cannot be longer than %max characters but is currently %length characters long.', [
        '@name' => empty($elements['#title']) ? $elements['#parents'][0] : $elements['#title'],
        '%max' => $elements['#maxlength'],
        '%length' => mb_strlen($elements['#value']),
      ]));
    }

If I only hit "return" 20 times in a textarea with #maxlength 20 and submit the form, then the value of $elements['#value'] is "\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n\r\n", and mb_strlen($elements['#value']) is 40.

I think best way forward is to do the same newline normalization in Drupal\Core\Render\Element\Textarea::valueCallback(), so that the maxlength validation is equivalent on the frontend and backend. I think it also makes sense to store this normalized value, so that there isn't an error if the same character limit is applied on the database column, but it needs to be validated that when the value is retrieved and presented back to browsers, that this doesn't create any issues across browsers/OSes.

Updated the issue summary with the appropriate solution.

I implemented the prescribed fix per the HTML5 standard. The FunctionalJavascript test was reverted in favor of Unit test cases in the existing TextareaTest.

Because of the browser behavior described in #19, I think it makes sense to include a Browser test after all. I think it'd also make sense to test that the saved value(s) do not exceed DB column sizes, so I think the test could look like this:

• Create an content entity type with a text field (Drupal\text\Plugin\Field\FieldType\TextItem) and set the maxlength storage setting to something low
• Visit entity create form and submit the form where the text field value length should equal the maxlength. This actually should be done three times, with "\r\n", "\n", and "\r" separately included in the text value
• Save should occur without a database exception
• Load the entity from database storage after save and confirm that the text contains "\n" and not "\r\n" or "\r"

@godotislate this issue is about textarea elements. It looks like core's fields with textarea elements don't have settings for maxlength attributes. Am I missing something?

Ah right, I'd forgotten that the textarea widget isn't used on text field items. Maybe the unit test is enough, so putting to RTBC.