Skip to content

[Security] Add more tests for StringUtils::equals #11808

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

Merged
merged 1 commit into from
Sep 2, 2014

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Aug 31, 2014

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

More tests for StringUtils::equals.

@jderusse
Copy link
Member

Can you use a dataprovider instead of multiples asserts?

@dunglas
Copy link
Member Author

dunglas commented Aug 31, 2014

👍

@dunglas
Copy link
Member Author

dunglas commented Sep 1, 2014

@fabpot The use of the short array notation (introduced in PHP 5.4) is a bug in fabbot?

@jderusse
Copy link
Member

jderusse commented Sep 1, 2014

@dunglas Symfony should be compatible with php 5.3

@dunglas
Copy link
Member Author

dunglas commented Sep 1, 2014

@jeremy-derusse yes I know, I'm talking about the error reported by fabbot: http://fabbot.io/report/symfony/symfony/11808/f9c08a32747428212be3f10a4deccf00815fd017

@fabpot
Copy link
Member

fabpot commented Sep 1, 2014

@dunglas I've just removed the check for the short array syntax. The report is more relevant now.

@dunglas dunglas force-pushed the fix_stringutilstest branch from f9c08a3 to a676863 Compare September 1, 2014 11:47
@dunglas
Copy link
Member Author

dunglas commented Sep 1, 2014

@fabpot thanks. CS fixed.

@fabpot
Copy link
Member

fabpot commented Sep 1, 2014

👍

1 similar comment
@stof
Copy link
Member

stof commented Sep 2, 2014

👍

@fabpot
Copy link
Member

fabpot commented Sep 2, 2014

Thank you @dunglas.

@fabpot fabpot merged commit a676863 into symfony:2.4 Sep 2, 2014
fabpot added a commit that referenced this pull request Sep 2, 2014
This PR was merged into the 2.4 branch.

Discussion
----------

[Security] Add more tests for StringUtils::equals

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

More tests for `StringUtils::equals`.

Commits
-------

a676863 [Security] Add more tests for StringUtils::equals
@fabpot
Copy link
Member

fabpot commented Sep 3, 2014

reverting as not all those tests actually pass.

fabpot added a commit that referenced this pull request Sep 3, 2014
…s (dunglas)"

This reverts commit 8fdfb6f, reversing
changes made to e99dfdf.
fabpot added a commit that referenced this pull request Sep 3, 2014
* 2.4:
  Revert "minor #11808 [Security] Add more tests for StringUtils::equals (dunglas)"
fabpot added a commit that referenced this pull request Sep 3, 2014
* 2.5:
  Revert "minor #11808 [Security] Add more tests for StringUtils::equals (dunglas)"
@dunglas
Copy link
Member Author

dunglas commented Sep 3, 2014

000bd0d / #11574 (not merged when the PR has been open) change the behavior of the method (remove the implicit cast of $userInput to string).

@fabpot @Tobion what's the best? To test only string and remove the cast in #11822 or fix 000bd0d?

@dunglas dunglas deleted the fix_stringutilstest branch December 5, 2015 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants