A bug in SqlManager.php:115: disableIdentifierQuoting = "false" (string) is equal to boolean true #323

Closed
kadishmal opened this Issue Nov 30, 2012 · 5 comments

4 participants

@kadishmal

tests/Fixtures/bookstore/build.properties.dist file contains this property called disableIdentifierQuoting which I set to be false, i.e. it must quote identifiers (CUBRID case).

Now, there is src/Propel/Generator/Manager/SqlManager.php:115

if ($this->getGeneratorConfig()->getBuildProperty('disableIdentifierQuoting')) {
    $platform->setIdentifierQuoting(false);
}

For the sake of testing, if you change the above to:

$disable = $this->getGeneratorConfig()->getBuildProperty('disableIdentifierQuoting');
// now $disable = "false" as I set in the build.properties.dist

// however string "false" is evaluated as boolean true,
// therefore, no matter what value you set, the $platform will also disable identifier quoting.
if ($disable) {
    $platform->setIdentifierQuoting(false);
}

Solutions:

  1. Either test if ($disable && $disable == 'true').
  2. Or in getBuildProperties() function change line 57 & 58:
                $pos = strpos($line, '=');
                $properties[trim(substr($line, 0, $pos))] = trim(substr($line, $pos + 1));

... to:

                $pos = strpos($line, '=');
                $prop = trim(substr($line, 0, $pos));
                $propVal = trim(substr($line, $pos + 1));

                if ($propVal == 'true')
                {
                    $propVal = true;
                }
                else if ($propVal == 'false')
                {
                    $propVal = false;
                }

                $properties[$prop] = $propVal;

The bug is found when adding CUBRID Database support in issue #317.

@willdurand
Propel member
@marcj
Propel member

filter_var($val, FILTER_VALIDATE_BOOLEAN) should do the trick in propel2.

@willdurand willdurand was assigned Jun 5, 2013
@marcj
Propel member

We should fix that issue after #527 has placed so we don't do things twice.

@marcj marcj modified the milestone: 2.0.0 beta, alpha-3 Apr 12, 2014
@mpscholten
Propel member

@marcj can this be closed? the new config feature should have fixed this.

@marcj
Propel member

Yes sir.

@marcj marcj closed this Aug 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment