Problem/Motivation

If a pattern ends with whitespace (eg. by an incorrect copy-paste including a TAB character), it's accepted – but the generated alias will not work.

Proposed resolution

Don't allow the patterns ending with whitespace.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#15 pathauto-dont_accept_whitespace_in_patterns-2818273-15.patch2.22 KBrahul.nahar001
#7 pathauto-dont_accept_whitespace_in_patterns-2818273-7.patch1.37 KBrahul.nahar001
#4 pathauto-dont_accept_whitespace_in_patterns-2818273-4.patch742 bytesrahul.nahar001
#2 pathauto-dont_accept_whitespace_in_patterns-2818273-1.patch753 bytesrahul.nahar001
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Boobaa created an issue. See original summary.

rahul.nahar001’s picture

rahul.nahar001 CreditAttribution: rahul.nahar001 at TATA Consultancy Services for Pfizer, Inc. commented
FileSize
pathauto-dont_accept_whitespace_in_patterns-2818273-1.patch753 bytes

Hi @Boobaa,

Yes, you are right for strong URL we need to validate patterns not ending with whitespace.
I have created a patch for fixed this bug.

Thanks
Rahul Nahar

Boobaa’s picture

Boobaa
he/him
CreditAttribution: Boobaa at Pronovix commented 
  1. +++ b/pathauto.module
@@ -195,6 +195,10 @@ function pathauto_pattern_validate($element, FormStateInterface $form_state) {
+    ¶

    Trailing space. 

  2. +++ b/pathauto.module
@@ -195,6 +195,10 @@ function pathauto_pattern_validate($element, FormStateInterface $form_state) {
+    if(ereg('[ \f\t\v]+$', $element['#value'])){

    Coding standards says if ( – ie. there should be a space between if and the parenthesis. 

  3. +++ b/pathauto.module
@@ -195,6 +195,10 @@ function pathauto_pattern_validate($element, FormStateInterface $form_state) {
+    }    ¶

    Trailing space.

And some tests would be more than welcome.

rahul.nahar001’s picture

rahul.nahar001 CreditAttribution: rahul.nahar001 at TATA Consultancy Services for Pfizer, Inc. commented
FileSize
pathauto-dont_accept_whitespace_in_patterns-2818273-4.patch742 bytes

Please find updated patch file.

Boobaa’s picture

Boobaa
he/him
CreditAttribution: Boobaa at Pronovix commented 
  1. +++ b/pathauto.module
@@ -195,6 +195,10 @@ function pathauto_pattern_validate($element, FormStateInterface $form_state) {
+    if (ereg('[ \f\t\v]+$', $element['#value'])) {

    ereg() is deprecated as of PHP 5.3.0. Please use preg_match() instead. 

  2. +++ b/pathauto.module
@@ -195,6 +195,10 @@ function pathauto_pattern_validate($element, FormStateInterface $form_state) {
+       $form_state->setError($element, t('The %element-title don\'t allow the patterns ending with whitespace.', array('%element-title' => $title)));

    Too much indentation. Use "doesn't" instead of "don't".

Some automated tests would be needed, too.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Active » Needs work
rahul.nahar001’s picture

rahul.nahar001 CreditAttribution: rahul.nahar001 at TATA Consultancy Services for Pfizer, Inc. commented
Status: Needs work » Active
FileSize
pathauto-dont_accept_whitespace_in_patterns-2818273-7.patch1.37 KB

Hi,
Please find updated patch file.
I am working on to create automated test case.

Thanks

Dinesh18’s picture

Dinesh18 CreditAttribution: Dinesh18 as a volunteer and at TATA Consultancy Services for Pfizer, Inc. commented

I have applied the patch and it is working as expected. It is throwing an error if there is a trailing white space.

Dinesh18’s picture

Dinesh18 CreditAttribution: Dinesh18 as a volunteer and at TATA Consultancy Services for Pfizer, Inc. commented
Status: Active » Needs review

Changed status to Needs Review

The last submitted patch, 2: pathauto-dont_accept_whitespace_in_patterns-2818273-1.patch, failed testing.

The last submitted patch, 2: pathauto-dont_accept_whitespace_in_patterns-2818273-1.patch, failed testing.

The last submitted patch, 4: pathauto-dont_accept_whitespace_in_patterns-2818273-4.patch, failed testing.

The last submitted patch, 4: pathauto-dont_accept_whitespace_in_patterns-2818273-4.patch, failed testing.

naiduharish’s picture

naiduharish CreditAttribution: naiduharish as a volunteer and at TATA Consultancy Services for Pfizer, Inc. commented

This Patch Works as expected !!

rahul.nahar001’s picture

rahul.nahar001 CreditAttribution: rahul.nahar001 at TATA Consultancy Services for Pfizer, Inc. commented
FileSize
pathauto-dont_accept_whitespace_in_patterns-2818273-15.patch2.22 KB

Hi @Boobaa,

Please find updated patch file with the automated test case.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Fixed

Thanks, committed.

LpSolit’s picture

LpSolit CreditAttribution: LpSolit as a volunteer commented

Wouldn't it be better to silently remove extra whitespaces instead of throwing an error?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Not sure, whould have been possible as well. You could provide a patch for that in a follow-up issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.