[PATCH] OCI8 module - proposed patches (PART 1 of 2)

Nice patches, but could you please provide ones without
the whitespace changes? Now it's not very clear what you
have actually changed..

--Jani

Hi, I had to split the original message in two e-mails because it was
longer than 30000 bytes.
In this first message you'll find the commentary and the first
attachment, the second message contains the second attachment.

Massimo

---

---

Hi all,

since posting bug #22674 resulted in no feedback to date and I believe
OCI8 persistent session management should quickly be fixed, I studied
the sources and came up with a solution to that particular bug and more.

The solutions have been working for a week now on two PHP 4.2.2 and two
PHP 4.3.1 servers, so this weekend I downloaded the current CVS snapshot
and applied them there.

Here follows a description of the problems and the proposed patches:

1. [Solves bug #22674, part 1] The sequence OCIPLOGON('user1', 'pwd1',
   'db1') + OCILOGON('user2', 'pwd2', 'db1') creates two persistent session
   handles instead of one, because the OCILOGON session wrongly "inherits"
   the persistency attribute of the OCIPLOGON session; BOTH sessions can be
   reused. I fixed one assignment statement in oci_do_connect.

2. [Solves bug #22674, part 2] The sequence OCIPLOGON + OCILOGON +
   OCINLOGON (same dbname for all, same userid and password for the latter
   2 calls) still creates two persistent session handles instead of one,
   but the session structure set up by OCILOGON is incorrectly freed when
   the module cleans up the OCINLOGON structures, so that while Oracle
   still sees the surplus session PHP cannot reuse it in subsequent
   scripts. This leads to a session build-up with Oracle eventually giving
   up. The problem lies in the way hashed_details is built: it is the same
   for OCILOGON and OCINLOGON, so when zend_hash_del is called during
   cleanup it deletes BOTH structures. I re-defined the hashed_details for
   OCINLOGON using gettimeofday(), so each OCINLOGON structure is now
   unique and different from any OCILOGON structure.

These first two patches are in the attached twobugs.txt file; if they
are accepted bug #22674 may be closed.

I also attach a fourpatches.txt file, which contains the above solutions
and two more patches I'd like to submit to your attention, since I
believe they could help make the module more flexible:

3. More often than not Oracle installations do NOT provide for a
   user-defined "password verification function", so that userid, password
   and dbname are treated case-insensitively by the RDBMS, while the OCI8
   module generates hashed_details that are case-sensitive. If the
   programmers mix case during development this leads to an excessive
   number of persistent sessions to Oracle. I introduced a php_ini boolean
   variable credentials_toupper which, when true (defaults to false),
   triggers uppercase conversion of userid, password and dbname before
   their use in oci_do_connect.

4. When the web servers are behind a firewall in a DMZ, the firewall
   truncates sessions when they reach a configurable but unavoidable
   inactivity timeout. This invalidates persistent sessions and in my
   experience (if the firewall doesn't specifically support SQL*NET) can't
   always be circumvented by a decrease in the tcp_keepalive_time; also,
   I'd rather leave such parameters at their default and let the firewall
   do its work which is dictated by security concerns. Idle persistent
   sessions are useless by themselves and should anyway be closed as soon
   as feasible to minimize load on the RDBMS. In past client-server (OS/2)
   applications I came up with a thread-based solution which automatically
   closed the client's Oracle connection after a configurable inactivity
   timeout, but since I don't know if this should be done in the current
   PHP implementation I simply decided to add a timestamp field in the
   oci_server structure and check in the PHP_RINIT_FUNCTION if a php_ini
   configurable connection_timeout (defaults to -1, or no timeout) has
   expired. In the affirmative, the expired server handle and all
   associated session handles are correcly closed. Since this happens at
   request init I am sure the check triggers for all PHP scripts, even
   though they don't access the database, as this helps to keep the number
   of connections down. On the other hand a heavily accessed server will
   never timeout persistent connections. My tests indicate that a 300s
   timeout is high enough to guarantee peak Oracle performance while being
   low enough to never create firewall problems. Especially when coupled
   with the solutions to bug #22674, the overhead induced by the up-front
   check is negligible.

As stated above, two new php_ini entries are proposed as follows:

[OCI8]
;if true, internally convert userid, password and dbname to uppercase
;oci8.credentials_toupper = false

;inactivity timeout for persistent connections (seconds). -1 means no
timeout.
;oci8.connection_timeout = -1

Hope this helps

Massimo Squillace

--
<- For Sale!

Hi, I had to split the original message in two e-mails because it was
longer than 30000 bytes.
In this first message you'll find the commentary and the first
attachment, the second message contains the second attachment.

as jani said, i will have a look at them once you resend them
without whitespace changes.

regards and thanx for your efffords!

re,
tc