cas_ldap_tokens() runs all values through check_plain(). When it encounters a binary value, such as a GUID / objectguid, you will get this error message:

Warning: htmlspecialchars(): Invalid multibyte sequence in argument in check_plain() (line 1545 of .../includes/bootstrap.inc).

The module should clean up strings which are not valid UTF-8 before running them through check_plain().

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Liam Morland’s picture

Status: Active » Needs review
FileSize
1.58 KB

Fixed attached.

lily_yanhong’s picture

Status: Needs review » Reviewed & tested by the community

I did test. It works for me. Thanks, Liam

Olarin’s picture

Thanks for submitting a patch. It looks pretty clean, but if you'll forgive a stupid question or two before I commit it, since I don't use the cas_ldap sub-module myself - your patch is currently only addressing GUIDs, and it's assuming any non-utf8-valid data of 32 characters is a GUID. Is there any other sort of binary data that might commonly be coming thru in an LDAP attribute that we should potentially be addressing while we're at it?

Liam Morland’s picture

Yes, it does make the assumption that any 32 character binary data can be formatted as a GUID. Currently, such data is replaced with empty string, so it's broken anyway. I don't know of any 32 character binary data besides a GUID which would be likely to come from LDAP.

The formatting part could be left out and that would still allow the binary data to be available and avoid the error message. Formatting means GUIDs look like they do in most other interfaces. It could check the name of the field and only format a GUID fields.

bfroehle’s picture

An alternative approach, of course, would be to add token mechanism to do the conversion --- for example something like
[cas:ldap:guid:guid_to_string].

bfroehle’s picture

Probably should also follow the discussion at #1533492: LDAP *: Need method for dealing with binary attributes, since we inherently rely upon Drupal's LDAP module to fetch the data. If they had proper support for binary attributes we could properly support them here.

Liam Morland’s picture

Makes sense. In the meantime, any binary attribute will be turned into the empty string and an warning message will be raised, even if PHP's display_errors is turned off. Because of this, at least bin2hex() should be called on data that doesn't pass drupal_validate_utf8().

bfroehle’s picture

Yes, I agree!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: cas_attributes_1929418_clean_binary.patch, failed testing.

Liam Morland’s picture

Issue summary: View changes
Status: Needs work » Reviewed & tested by the community

The errors causing the test to fail are inside the phpCAS library, not in the CAS Attributes module, so this patch is still ready to be committed.

DamienMcKenna’s picture

yalet’s picture

Status: Reviewed & tested by the community » Fixed

Committed.

  • yalet committed 44ec7de on 7.x-1.x authored by Liam Morland
    Issue #1929418 by Liam Morland: Don't run binary data, such as GUID,...

Status: Fixed » Closed (fixed)

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