Contrib.social

Undefined varibles in install.inc

Open on Drupal.org →
Created on 3 October 2014, over 9 years ago
Updated 22 February 2023, over 1 year ago

Hi everyone.

First of all this is my first issue report identified at Amsterdam DrupalCon 2014.

In core/includes/install.inc I have identified several unidentified variables in the function drupal_rewrite_settings.
While these variables might(probably) get set through the process of the foreach loop the variables should still have some default values.

This mitigates false positives from static code analysis made by IDE's etc.

The first issue:

        case 'right_bracket':
            if ($value == ']') {
              if (isset($current[$index])) {

The index variable is not set outside the scope and not defined in the case 'right_bracket'.

Second issue:

          case 'candidate_right':
            if (_drupal_rewrite_settings_is_simple($type, $value)) {
              $value = _drupal_rewrite_settings_dump_one($current);
              // Unsetting $current would not affect $settings at all.
              unset($parent[$index]);

Again three variables is used without being defined first or outside the scope of the switch statement.
The three variables are $current, $index and $parent.

Beta phase evaluation

<!--Uncomment the relevant rows for the issue. -->
📌 Task
Status

Active

Version

9.5

Component
Install  →

Last updated 1 day ago

No maintainer
Created by

🇩🇰Denmark RonnieJ

Live updates comments and jobs are added and updated live.
  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

    Still seems to be relevant but the bulk of the code was moved to SettingsEditor.php

    Tagged for novice for beginner developers.

  • Comment over 1 year ago →
    🇮🇳India _utsavsharma

    Patch for 10.1.x.

  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India Aadhar_Gupta

    Applying fix for 10.1.x and fixing custom commands

  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    Not sure what happened but that patch is full of out of scope changes. Can compare the files sizes between the two and see

  • Status changed to Active over 1 year ago
  • Comment over 1 year ago →
    🇳🇿New Zealand quietone New Zealand

    According to #14, this is not a novice issue. I am removing the tag. Setting to active since this needs a re-think.

  • Comment over 1 year ago →
    🇬🇧United Kingdom alexpott 🇪🇺🌍

    We also need a new issue summary and title given this now has nothing to do with install.inc

    I think rather than focussing on the unused variables we should focus on the design of \Drupal\Core\Site\SettingsEditor and see if we can improve that. And as part of that solve the "variable might not be defined" warnings in the class.

    Also this really is a task and not a bug.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024