Tuesday, January 3, 2017

Learn to hate repetition - a lesson from APEX LOVs

If I was forced to pick just one piece of advice any developer should follow when writing code it would be: 

Avoid Repetition!
a.k.a, DRY - Don't Repeat Yourself
a.k.a., SPOD - Single Point of Definition

When the same code (business rule, formula, "magic" value, SQL statement, etc.) appears in more than one place, you create opportunities for bugs to creep into your code. How? You fix the problem in one place, but what about all the other places?

Here's an example of repetition in the PL/SQL Challenge APEX application and how I was able to get rid of the redundancy. 

I needed to add an LOV (list of values) to an item. I soon discovered that we had three LOVs already defined that were very similar to what I needed:



Now, just looking at the names of those LOVs made me shudder. They differ only by what appears to be the page number on which they are used. That didn't make much sense to me. So I drilled down in an attempt to gain clarify, and found that I was, indeed, correct. The only differences between these LOVs was the use of a different page item in the WHERE clause.


I had been using APEX for well over a year now by this time, but I still consider myself a novice and certainly lack an understanding of many of the nuances and limitations of the tool. Still, my quick glance at this situation had me thinking as follows:

1. It really would be better if page-specific item references were not stuck inside a named LOV. These LOVs do not "live" inside a single page and can, theoretically, be used across an entire application.

2. Surely there's got to be a way to generalize the query so that I can have a single LOV that can be used in all these locations.

So I clicked on the link in one of the LOVs to get some help and found:


Well, gee, that looks like a very useful approach. So I created a new LOV that does not contain the page number in its name, and does not contain a hard-coded page item reference in the query:


Now the only requirement for using this LOV is that the name of the item contain the domain ID is of the form:

:PNNN_domain_id

where NNN is the page number. So it's not quite completely generic, but it's a lot closer than before, and I was able to replace three LOVs with just one.

Goodbye (the worst of the) hardcoding, goodbye repetition!

And another nice reminder of how easy it is to build and execute dynamic SQL statements via PL/SQL. And you can see here, I changed a static query to a PL/SQL block that returns a string. That string will then be executed by the APEX engine via an EXECUTE IMMEDIATE call.

Note also that even though I concatenate text to put together my where clause, I do not introduce a SQL injection vulnerability. The :app_page_id bind variable is set by APEX itself. And the end result of the concatenation is a string that contains a bind variable.

Resources

Oracle Application Express (v5.1 was just released last month!)
Get Rid of Hard Coding in PL/SQL (a Practically Perfect PL/SQL YouTube playlist)

11 comments:

  1. dear sir, if you have three LOV item in the same page with different domain id. how would you solve it?

    ReplyDelete
    Replies
    1. Generally, you need some way in the LOV to determine which "path" to follow. Having said that, I am not all that much of an APEX pro and so I hereby direct you over to the OTN APEX forum. https://community.oracle.com/community/database/developer-tools/application_express

      Delete
    2. dear sir,
      thanks alot and one more thing
      how to write plsql function that return sql query string dynamicaly in plsql and assert there is no sql injection

      Delete
    3. Do you mean test the string to see if it *could* be a target for injection? The string itself generally isn't vulnerable. It's in the process of CONSTRUCTING the string - with concatenations - that vulnerability is introduced.

      Delete
    4. yes sir i meant the process of constructing the string
      PLEASE see this code

      CREATE OR REPLACE package TEST_PK is

      G_VAR VARCHAR2(500):= NULL;

      end TEST_PK;
      /
      -------------------------------------------
      set serveroutput on;
      DECLARE

      LC_QRY_SELECT CONSTANT VARCHAR2(500) := q'#
      BEGIN
      TEST_PK.G_VAR := 'SELECT DOMAIN_NAME AS D, DOMAIN_VALUE AS R FROM DOMAIN_TABLE
      WHERE DOMAIN_VALUE = :X' ;
      END; #';

      L_DOMAIN_VALUE varchar2(50):='GENDER';

      BEGIN
      EXECUTE IMMEDIATE LC_QRY_SELECT USING L_DOMAIN_VALUE ;
      DBMS_OUTPUT.PUT_LINE(TEST_PK.G_VAR);
      END;

      THE RESULT IS Error at line 2
      ORA-01006: bind variable does not exist
      ORA-06512: at line 17

      Delete
    5. Tricky, tricky! You have put the placeholder (:X) inside the dynamically constructed SQL statement, which is inside a dynamic PL/SQL block. The USING clause looks for a placeholder in the PL/SQL code and finds none - that :X is just text inside a string.

      Delete
    6. dear sir,
      if i but :x bind varible out side then its exposed to sql injection.
      see the example
      set serveroutput on;
      DECLARE

      LC_QRY_SELECT CONSTANT VARCHAR2(500) := q'#
      BEGIN
      TEST_PK.G_VAR := 'SELECT DOMAIN_NAME AS D, DOMAIN_VALUE AS R FROM DOMAIN_TABLE
      WHERE DOMAIN_VALUE = '||:X ;
      END; #';

      L_DOMAIN_VALUE varchar2(50):='''GENDER'' OR 1=1';

      BEGIN
      EXECUTE IMMEDIATE LC_QRY_SELECT USING L_DOMAIN_VALUE ;
      DBMS_OUTPUT.PUT_LINE(TEST_PK.G_VAR);
      END;

      in Apex you solved the LOV proplem using function that returns a SQL query ,
      how they did it?

      Delete
    7. Perhaps we should take a step back. What are you trying to achieve? For example, why are you executing a dynamic PL/SQL block with a dynamic SELECT inside it? There is no reason to do that. In other words, why not something like this - no concatenation, no injection:

      DECLARE
      str CONSTANT VARCHAR2 (500)
      := '
      SELECT n FROM my_data WHERE s LIKE :my_s';

      l_n my_data.n%TYPE;
      BEGIN
      EXECUTE IMMEDIATE str INTO l_n USING IN 'a%';

      DBMS_OUTPUT.put_line (l_n);
      END;
      /

      Delete
    8. The APEX_PLUGIN package provides the interface declarations and some utility
      functions to work with plug-ins.

      Apex allow you to create new item as plugin imagin that you have
      new item with one more property called DOMIN VALUE: so you can insert your DOMIN value (:X) in this
      property so your proplem is solved.
      i am almost finishing it. if you want to see it i will send you a copy of it and we can improve it
      and share it with others as Open source plugin in https://apex.world/ords/f?p=100:700:::NO:::.

      i think i found the solution could you please see it
      set serveroutput on;
      DECLARE

      LC_QRY_SELECT CONSTANT VARCHAR2(500) := q'#
      BEGIN
      TEST_PK.G_VAR := 'SELECT DOMAIN_NAME AS D, DOMAIN_VALUE AS R FROM DOMAIN_TABLE
      WHERE DOMAIN_VALUE = '||SYS.DBMS_ASSERT.ENQUOTE_LITERAL (:X);
      END; #';

      L_DOMAIN_VALUE varchar2(50):='GENDER' ||''' OR 1=1''';
      --'''GENDER'' OR 1=1'
      BEGIN
      EXECUTE IMMEDIATE LC_QRY_SELECT USING L_DOMAIN_VALUE ;
      DBMS_OUTPUT.PUT_LINE(TEST_PK.G_VAR);
      END;

      Delete
  2. Steven, good point. Are you still using APEX 4.x? At least your screenshots are showing it.

    Denes Kubicek

    ReplyDelete
    Replies
    1. Ha, good call, Denes. No, we are on 5.1 and loving it. That was, in essence, a "copy and paste" error from something I'd written earlier.

      Delete