diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

merged branch egeloen/password-type (PR #6007)

Browse Source 
This PR was merged into the master branch.

Commits
-------

97f6a1b [Form] Update password type trimming to false

Discussion
----------

[Form] Update password trimming to false by default

Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: -
License of the code: MIT
Documentation PR: ~

Hey!

Today, I realize that the password type is by default trimmed. IMHO, this is not the expected behavior. By default, the password type should not trim the input value.

Regards

---------------------------------------------------------------------------

by nomack84 at 2012-11-13T19:16:29Z

👍

---------------------------------------------------------------------------

by mvrhov at 2012-11-13T19:57:29Z

IMHO password and username fields should be trimmed. whitespace at the beginning and at the end of those fields are not wanted. At least I don't want to deal with a user support where WS on those fields is not trimmed.

---------------------------------------------------------------------------

by egeloen at 2012-11-13T20:08:08Z

@mvrhov I agree with you about username fields and other "text" fields but in case of a password field, if the end user specifies white space at the begin/end of his password, it should not be trimmed. It should simply let it as it is. I open this PR due to two customers who reports me this behavior.

---------------------------------------------------------------------------

by clemherreman at 2012-11-14T10:06:15Z

@mvrhov I agree, username shouldn't be trimmed, however password are kind of special. They should be used *"as is"*, as lots of users have wicked passwords.

Moreover, usually the password is asked twice, so if there are spaces, they are most likely wanted by the end user.

So 👍

---------------------------------------------------------------------------

by clemherreman at 2012-11-14T10:07:27Z

Also Travis status on this PR is **failed** because of an error when downloading the deps.

---------------------------------------------------------------------------

by geoffrey-brier at 2012-11-14T10:34:56Z

👍

---------------------------------------------------------------------------

by bschussek at 2012-11-14T15:01:43Z

Could you please add a test case to PasswordTypeTest?

Please also reference this PR in the test

(= add the comment `// https://github.com/symfony/symfony/pull/6007` before the test)

---------------------------------------------------------------------------

by egeloen at 2012-11-14T15:10:36Z

@bschussek I have updated the PR.

---------------------------------------------------------------------------

by bschussek at 2012-11-14T15:24:34Z

Thanks! Could you please squash the commits?

---------------------------------------------------------------------------

by egeloen at 2012-11-14T15:30:11Z

@bschussek Done.

---------------------------------------------------------------------------

by stloyd at 2012-11-14T15:39:47Z

Should this be noted in `UPGRADE` file ? (as this is change of actually BC break =))

---------------------------------------------------------------------------

by egeloen at 2012-11-15T22:59:45Z

@stloyd Where can I put it? In the [UPGRADE-2.2](https://github.com/symfony/symfony/blob/master/UPGRADE-2.2.md) file?

---------------------------------------------------------------------------

by stloyd at 2012-11-15T23:02:51Z

@egeloen IMO yes, according this will go to `master` (which is actual _dev_ branch for `2.2`)

---------------------------------------------------------------------------

by egeloen at 2012-11-16T13:54:04Z

@fabpot I have removed the comment & added an entry in the `UPGRADE-2.2` file.
This commit is contained in:
Fabien Potencier 2012-11-16 15:57:52 +01:00
commit 68ae207002
3 changed files with 14 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
UPGRADE-2.2.md
View File

@ -31,3 +31,7 @@
$accepts = AcceptHeader::fromString($request->headers->get('Accept'))->all();
```
### Form
* The PasswordType is now not trimmed by default.

1
src/Symfony/Component/Form/Extension/Core/Type/PasswordType.php
View File

@ -35,6 +35,7 @@ class PasswordType extends AbstractType
{
$resolver->setDefaults(array(
'always_empty' => true,
'trim' => false,
));
}

9
src/Symfony/Component/Form/Tests/Extension/Core/Type/PasswordTypeTest.php
View File

@ -39,4 +39,13 @@ class PasswordTypeTest extends TypeTestCase
$this->assertSame('pAs5w0rd', $view->vars['value']);
}
public function testNotTrimmed()
{
$form = $this->factory->create('password', null);
$form->bind(' pAs5w0rd ');
$data = $form->getData();
$this->assertSame(' pAs5w0rd ', $data);
}
}