Skip to content
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

fix notices in xdsamlauth #19

Merged
merged 2 commits into from
Jan 13, 2017
Merged

Conversation

plessbd
Copy link
Contributor

@plessbd plessbd commented Jan 9, 2017

Description

notices are thrown if the idp does not have the right indexes

Motivation and Context

Dont want to throw notices if they are not needed

Tests performed

Verified that the notices were no longer shown when display_errors = On in php.ini

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project as found in the CONTRIBUTING document.

notices are thrown if the idp does not have the right indexes
@plessbd plessbd added the bug Bugfixes label Jan 9, 2017
@plessbd plessbd added this to the v6.5.0 milestone Jan 9, 2017
@plessbd plessbd changed the base branch from xdmod6.5 to xdmod6.6 January 10, 2017 18:17
@plessbd plessbd modified the milestones: v6.6.0, v6.5.0 Jan 10, 2017
@@ -113,10 +113,10 @@ public function getLoginLink()
$orgDisplay = "";
$icon = "";
foreach ($idpAuth as $idp) {
if ($idp['OrganizationDisplayName']) {
if (array_key_exists('OrganizationDisplayName', $idp) && !empty($idp['OrganizationDisplayName'])) {
Copy link
Member

Choose a reason for hiding this comment

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

the array_key_exists is redundant as the empty() built-in does an isset() check and does not give a warning if the variable is not set. See e.g. http://php.net/manual/en/function.empty.php

changes per code review
@plessbd plessbd merged commit 2c0fe9b into ubccr:xdmod6.6 Jan 13, 2017
@plessbd plessbd deleted the fix-xdsamlauth-notices branch January 13, 2017 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugfixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants