Using Shield 8.x-1.x, I've got a notice when accessing for the first time, and get the basic auth form.

Notice : Undefined offset: 1 in Drupal\shield\ShieldMiddleware->handle() (line 69 of /web/modules/contrib/shield/src/ShieldMiddleware.php).

CommentFileSizeAuthor
#10 shield-undefined-offset-2794899-10.patch937 bytesbadjava
#5 shield_undefined_offset-2794899-5.patch1.02 KBflocondetoile
#3 shield_undefined_offset-2794899-3.patch1.02 KBflocondetoile
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

flocondetoile created an issue. See original summary.

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
CreditAttribution: flocondetoile at Flocon de toile commented
Issue summary: View changes
flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
CreditAttribution: flocondetoile at Flocon de toile commented
Status: Active » Needs review
FileSize
shield_undefined_offset-2794899-3.patch1.02 KB

It's because for the first check if the visitor is not authenticated, the $request->server->has('HTTP_AUTHORIZATION') is present but empty.

the tiny patch attached fix these notices.

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
CreditAttribution: flocondetoile at Flocon de toile commented
Status: Needs review » Needs work

Seems it's not sufficient.

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
CreditAttribution: flocondetoile at Flocon de toile commented
Status: Needs work » Needs review
FileSize
shield_undefined_offset-2794899-5.patch1.02 KB

The good patch attached

flocondetoile’s picture

flocondetoile
Primary language French
Location Lyon
CreditAttribution: flocondetoile at Flocon de toile commented
swentel’s picture

swentel CreditAttribution: swentel commented

+1 - patch applies with offset, but does the trick.

badjava’s picture

badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commented

The patch fixes the issue but why do we even do the ->has checks on the two elseif statements? They are returning true but with an empty value so it seems pointless unless I am missing something.

swentel’s picture

swentel CreditAttribution: swentel commented

Makes sense, probably just an !empty() check is good enough.

badjava’s picture

badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commented
FileSize
shield-undefined-offset-2794899-10.patch937 bytes

Here is an updated patch with the changes discussed in #8 and #9.

badjava’s picture

badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commented
Priority: Normal » Major

Raising the priority of this issue since it's causing PHP errors on the stable release.

kmoll’s picture

kmoll CreditAttribution: kmoll at Appnovation for Pfizer, Inc. commented

I tested this patch and it fixes the php notice I get when testing it without the patch with the 8.x-1.x version.

kmoll’s picture

kmoll CreditAttribution: kmoll at Appnovation for Pfizer, Inc. commented
Status: Needs review » Reviewed & tested by the community

  • badjava committed 8a3fb81 on 8.x-1.x 
    Issue #2794899 by badjava, flocondetoile, kmoll, swentel: Notice :...
badjava’s picture

badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commented
Status: Reviewed & tested by the community » Fixed

Thanks everyone for helping fix this issue.

Status: Fixed » Closed (fixed)

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