Skip to content

[Ldap] Escape carriage returns in LDAP DNs. #16842

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

Closed
wants to merge 3 commits into from
Closed

[Ldap] Escape carriage returns in LDAP DNs. #16842

wants to merge 3 commits into from

Conversation

ChadSikorra
Copy link
Contributor

Depends upon this commit in polyfill: symfony/polyfill#14

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

Carriage returns are supposed to be escaped in a LDAP DN. Leading and trailing spaces should be encoded as well. The spaces were taken care of in the polyfill implementation of ldap_escape, but the actual PHP function doesn't do the same. So I moved that logic within the component function and removed it from the polyfill function.

@ChadSikorra ChadSikorra changed the title Escape carriage returns in LDAP DNs. [Ldap] Escape carriage returns in LDAP DNs. Dec 7, 2015
use Symfony\Component\Ldap\LdapClient;
use Symfony\Polyfill\Php56\Php56 as p;

class LdapClientTest extends \PHPUnit_Framework_TestCase
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should skip the test if the extension is not enabled, which is the case in one of the two Appveyor builds (one is run with the extension, the other without).

@ChadSikorra
Copy link
Contributor Author

I adjusted the test to require the LDAP extension. Seems to have appeased Appveyor anyway.

@csarrazi
Copy link
Contributor

LGTM 👍

@fabpot
Copy link
Member

fabpot commented Dec 18, 2015

Thank you @ChadSikorra.

fabpot added a commit that referenced this pull request Dec 18, 2015
This PR was squashed before being merged into the 2.8 branch (closes #16842).

Discussion
----------

[Ldap] Escape carriage returns in LDAP DNs.

Depends upon this commit in polyfill: symfony/polyfill#14

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

Carriage returns are supposed to be escaped in a LDAP DN. Leading and trailing spaces should be encoded as well. The spaces were taken care of in the polyfill implementation of `ldap_escape`, but the actual PHP function doesn't do the same. So I moved that logic within the component function and removed it from the polyfill function.

Commits
-------

2243db4 [Ldap] Escape carriage returns in LDAP DNs.
@fabpot fabpot closed this Dec 18, 2015
This was referenced Dec 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants