# [PATCH] OCI8 module - proposed patches (PART 1 of 2) - Thread: https://externals.io/message/762 - Original archive: https://news-web.php.net/php.internals/762 - Started by SQUILLACE MASSIMO on 2003-04-07T16:55:34+00:00 - Messages: 4 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #762 — SQUILLACE MASSIMO — 2003-04-07T16:55:34+00:00 URL: https://externals.io/message/762#762 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 --- ## #773 — Jani Taskinen — 2003-04-07T23:27:59+00:00 URL: https://externals.io/message/762#773 · In reply to: #762 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! --- ## #784 — Thies C. Arntzen — 2003-04-08T10:07:42+00:00 URL: https://externals.io/message/762#784 · In reply to: #762 > 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 --- ## #817 — Maxim Maletsky — 2003-04-09T01:42:37+00:00 URL: https://externals.io/message/762#817 · In reply to: #784 We have discussed these bugs/patches with Massimo (who is actually my co-worker) for already some time and his solution seems to be working fine. The only doubt I still have is whether it is ok to be adding the two new php.ini directives for OCI8. If no one objects about it, I can then apply and commit the patches. Thies? Maxim Maletsky maxim@php.net Thies C. Arntzen writes: > > > […] > > 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