Zend Framework

connect errors are supressed when dbname is provided

Details

  • Type: Improvement Improvement
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 1.8.0
  • Component/s: Zend_Db_Adapter_Oracle
  • Labels:
    None

Description

It seems a bit inconsistent that oci_connect() errors are suppressed in _connect() when dbname is provided in config.ini, but are not supressed when dbname isn't present.

It caused me a bit of head scratching during a recent connection issue.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Mickael Perraud added a comment - - edited

In this case, Zend_Db_Adapter_Abstract will throw an exception with _checkRequiredOptions():

Zend_Db_Adapter_Exception("Configuration array must have a key for 'dbname' that names the database instance")

Since SVN5014, this part of code is unused.

Show
Mickael Perraud added a comment - - edited In this case, Zend_Db_Adapter_Abstract will throw an exception with _checkRequiredOptions(): 
Zend_Db_Adapter_Exception("Configuration array must have a key for 'dbname' that names the database instance")
Since SVN5014, this part of code is unused.
Hide
Permalink
Steve Jordan added a comment -

sorry, I wasn't very clear in the description.

inside the _connect() function there is the following code:

Zend_Db_Adapter_Oracle::_connect() - partial
if (isset($this->_config['dbname'])) {
            $this->_connection = @oci_connect(
                $this->_config['username'],
                $this->_config['password'],
                $this->_config['dbname']);
        } else {
            $this->_connection = oci_connect(
                $this->_config['username'],
                $this->_config['password']);
        }

at the moment a check is made to see if dbname has been declared somewhere, and if it has oci_connect has it's errors suppressed (@).
There is no error suppression when dbname has not been set.

This is an inconsistency, and caused me some issues while trying to diagnose a connection issue (The exception thrown contained no useful information)

I hope this is clearer.

Show
Steve Jordan added a comment - sorry, I wasn't very clear in the description. inside the _connect() function there is the following code:
Zend_Db_Adapter_Oracle::_connect() - partial
if (isset($this->_config['dbname'])) {
            $this->_connection = @oci_connect(
                $this->_config['username'],
                $this->_config['password'],
                $this->_config['dbname']);
        } else {
            $this->_connection = oci_connect(
                $this->_config['username'],
                $this->_config['password']);
        }
at the moment a check is made to see if dbname has been declared somewhere, and if it has oci_connect has it's errors suppressed (@). There is no error suppression when dbname has not been set. This is an inconsistency, and caused me some issues while trying to diagnose a connection issue (The exception thrown contained no useful information) I hope this is clearer.
Hide
Permalink
Mickael Perraud added a comment -

You were clear, no problem .

Based on actual trunk, I think this code is unreachable. So we could replace:

if (isset($this->_config['dbname'])) {
    $this->_connection = @oci_connect(
        $this->_config['username'],
        $this->_config['password'],
        $this->_config['dbname']);
} else {
    $this->_connection = oci_connect(
        $this->_config['username'],
        $this->_config['password']);
}

by:

$this->_connection = @oci_connect(
    $this->_config['username'],
    $this->_config['password'],
    $this->_config['dbname']);

Because $this->_config['dbname'] is mandatory, if it misses, then you have an exception thrown by Zend_Db_Adapter_Abstract::_checkRequiredOptions() before entering in Zend_Db_Adapter_Oracle::_connect()

Show
Mickael Perraud added a comment - You were clear, no problem . Based on actual trunk, I think this code is unreachable. So we could replace: 
if (isset($this->_config['dbname'])) {
    $this->_connection = @oci_connect(
        $this->_config['username'],
        $this->_config['password'],
        $this->_config['dbname']);
} else {
    $this->_connection = oci_connect(
        $this->_config['username'],
        $this->_config['password']);
}
by: 
$this->_connection = @oci_connect(
    $this->_config['username'],
    $this->_config['password'],
    $this->_config['dbname']);
Because $this->_config['dbname'] is mandatory, if it misses, then you have an exception thrown by Zend_Db_Adapter_Abstract::_checkRequiredOptions() before entering in Zend_Db_Adapter_Oracle::_connect()
Hide
Permalink
Steve Jordan added a comment -

I see what your saying.
It sounds like a good idea to remove code which will never be run.

But the problem which I was having (with the empty Exception messages) will still occur in your replacement code, because you still have the @ suppressing error messages.

If you look at the next code block in the file:

Zend_Db_Adapter_Oracle::_connect() - partial
// check the connection
if (!$this->_connection) {
            /**
             * @see Zend_Db_Adapter_Oracle_Exception
             */
            require_once 'Zend/Db/Adapter/Oracle/Exception.php';
            throw new Zend_Db_Adapter_Oracle_Exception(oci_error());
        }
Show
Steve Jordan added a comment - I see what your saying. It sounds like a good idea to remove code which will never be run. But the problem which I was having (with the empty Exception messages) will still occur in your replacement code, because you still have the @ suppressing error messages. If you look at the next code block in the file:
Zend_Db_Adapter_Oracle::_connect() - partial
// check the connection
if (!$this->_connection) {
            /**
             * @see Zend_Db_Adapter_Oracle_Exception
             */
            require_once 'Zend/Db/Adapter/Oracle/Exception.php';
            throw new Zend_Db_Adapter_Oracle_Exception(oci_error());
        }
Hide
Permalink
Steve Jordan added a comment -

sorry, I hit "Add" too soon.

the call to oci_error() will never return anything while the oci_connect() errors are suppressed.

So my suggestion is to remove the @ before oci_connect.

Show
Steve Jordan added a comment - sorry, I hit "Add" too soon. the call to oci_error() will never return anything while the oci_connect() errors are suppressed. So my suggestion is to remove the @ before oci_connect.
Hide
Permalink
Mickael Perraud added a comment -

the call to oci_error() will never return anything while the oci_connect() errors are suppressed.

For me, it's false, see these examples:

<?php
oci_connect('user', 'good_password', 'database');
print_r(oci_error());

=> return nothing

<?php
oci_connect('user', 'bad_password', 'database');
print_r(oci_error());

=>

<br />
<b>Warning</b>:  oci_connect() [<a href='function.oci-connect'>function.oci-connect</a>]: ORA-01017: invalid username/password; logon denied in <b>D:\web\www\test_oracle.php</b> on line <b>2</b><br />
Array
(
    [code] => 1017
    [message] => ORA-01017: invalid username/password; logon denied
    [offset] => 0
    [sqltext] => 
)
<?php
@oci_connect('user', 'bad_password', 'database');
print_r(oci_error());

=>

Array
(
    [code] => 1017
    [message] => ORA-01017: invalid username/password; logon denied
    [offset] => 0
    [sqltext] => 
)

The goal of @ is simply to suppress the warning but oci_error() is always populated.

No?

Show
Mickael Perraud added a comment -
the call to oci_error() will never return anything while the oci_connect() errors are suppressed.
For me, it's false, see these examples: 
<?php
oci_connect('user', 'good_password', 'database');
print_r(oci_error());
=> return nothing 
<?php
oci_connect('user', 'bad_password', 'database');
print_r(oci_error());
=> 
<br />
<b>Warning</b>:  oci_connect() [<a href='function.oci-connect'>function.oci-connect</a>]: ORA-01017: invalid username/password; logon denied in <b>D:\web\www\test_oracle.php</b> on line <b>2</b><br />
Array
(
    [code] => 1017
    [message] => ORA-01017: invalid username/password; logon denied
    [offset] => 0
    [sqltext] => 
)
<?php
@oci_connect('user', 'bad_password', 'database');
print_r(oci_error());
=> 
Array
(
    [code] => 1017
    [message] => ORA-01017: invalid username/password; logon denied
    [offset] => 0
    [sqltext] => 
)
The goal of @ is simply to suppress the warning but oci_error() is always populated. No?
Hide
Permalink
Steve Jordan added a comment -

It looks like I have misunderstood the purpose of @

In which case the exception thrown needs to contain at least the code and message, something like:

// check the connection
if (!$this->_connection) {
            /**
             * @see Zend_Db_Adapter_Oracle_Exception
             */
            require_once 'Zend/Db/Adapter/Oracle/Exception.php';
            $errorMessageArray = oci_error();
            throw new Zend_Db_Adapter_Oracle_Exception($errorMessageArray[code] . ': ' . $errorMessageArray[message]);
        }

?

Show
Steve Jordan added a comment - It looks like I have misunderstood the purpose of @ In which case the exception thrown needs to contain at least the code and message, something like: 
// check the connection
if (!$this->_connection) {
            /**
             * @see Zend_Db_Adapter_Oracle_Exception
             */
            require_once 'Zend/Db/Adapter/Oracle/Exception.php';
            $errorMessageArray = oci_error();
            throw new Zend_Db_Adapter_Oracle_Exception($errorMessageArray[code] . ': ' . $errorMessageArray[message]);
        }
?
Hide
Permalink
Mickael Perraud added a comment -

Open Zend/Db/Adapter/Oracle/Exception.php, you will see how this is managed

Show
Mickael Perraud added a comment - Open Zend/Db/Adapter/Oracle/Exception.php, you will see how this is managed
Hide
Permalink
Steve Jordan added a comment -

I see. That makes sense now.

So in my case the lack of an error message is down to oci_error not returning one, rather than the framework's processing.

Thanks for pointing me in the right direction.

Show
Steve Jordan added a comment - I see. That makes sense now. So in my case the lack of an error message is down to oci_error not returning one, rather than the framework's processing. Thanks for pointing me in the right direction.
Hide
Permalink
Mickael Perraud added a comment -

The unreachable code was removed with SVN 14324 to close this issue.

Show
Mickael Perraud added a comment - The unreachable code was removed with SVN 14324 to close this issue.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Zend Framework. Try JIRA - bug tracking software for your team.