This internal order tracking application has a function that is passed an order number and order data to store and have it available to be looked up later. You would expect it stores it in some MySQL database or something? Wrong. It does this:

$orderstring = "case ".$oid.":\n\t\$order".$oid." = ".$odata.";\n\tbreak;";
$f = fopen('orders.inc', 'a');
fwrite($f, $orderstring);
fclose($f);

That should produce something like this if the order number is 5000:

switch($getorder) {
*snip*
case 5000:
$order5000 = "orderdata";
break;
}

That's right: the "look up order code" just does an include('orders.inc'); and runs the given order number through a gigantic switch() statement to see what the data is.

 Man, I like dymanic code but doing that is just wrong. It looks like what someone would do after the first hour of learning programming.

Well, if you ever wanted to move to a database, at least orders.inc should be easy to parse and convert to SQL insert statements.  The biggest problem with this is the memory usage and lack of concurrent reading/writing.  

I don't think it's horrible, if you think of include files as data stores, for which PHP has an automatic, native parser.  I wouldn't recommend it, though.  

Well, hard-coding the value makes it much more efficient than using a database. Right?

 

AccessGuru:

Well, if you ever wanted to move to a database, at least orders.inc should be easy to parse and convert to SQL insert statements.  The biggest problem with this is the memory usage and lack of concurrent reading/writing.  

I don't think it's horrible, if you think of include files as data stores, for which PHP has an automatic, native parser.  I wouldn't recommend it, though.  

First, speed. PHP processes switch statements line-by-line, so what you have is essentially a linear search. All new orders (which are the most likely to be accessed soon) are appended to the end of the file, so all older orders must be searched through first. And since PHP has to parse, check and execute the whole file for each query, the result is even less efficient than a flat-file database like SSDS.

Second, it only supports 2 basic functions: add one record, and lookup by order ID. You have to write a special parser if you want to know simple things like how many orders there are, or what the highest order ID is. Also deletion is probably best handled manually using a text editor.

Third, as it appears to be implemented, order ID doesn't have to be unique. What does PHP do with a switch with two of the same case? At best, you lose access to one order; at worst, it throws an error and the whole thing blows up. 

Self-modifying code was an exceedingly bad thing back in the 1980s when we had stand-alone micros like the TRS-80 and C-64. With a whole internet's worth of Nigerian scammers, it's an even worse idea to put user-input into your application's source code. What's the bet that this "internal order application" can be reached from outside? That's just opening up your wallet and saying "Take what you want."

Maybe I'm missing something but how does the } character get put in to finish the switch statement when new records are getting simply appended?

 Looks like the PHP counterpart to JSON.

fyjham:

Maybe I'm missing something but how does the } character get put in to finish the switch statement when new records are getting simply appended?

It's an included file, so you have something like this somewhere:

switch($getorder) {

    include('orders.inc');

}

 

ammoQ:

Looks like the PHP counterpart to JSON.

Not really.  It is possible to export arrays and objects in PHP directly to PHP source which can then be included elsewhere.  Using a switch statement is utterly stupid.  For certain infrequently-changed pieces of data, it makes a lot more sense to store them in PHP source files rather than a DB, memcached or XML.  The situation the OP is dealing with is retarded but it's certainly a misapplication of a useful feature. 

Zecc:

It's an included file, so you have something like this somewhere:

switch($getorder) {

    include('orders.inc');

}

 

You can't do an include inside of a switch statement or a class definition. 

morbiuswilters:

You can't do an include inside of a switch statement or a class definition. 

You're right.

Then it could be something like this (and this time I checked before posting):

eval( 'switch($order){' . file_get_contents('orders.inc') . '}' );

Ouch. This one kills even the stupidity of the Storray engine!

At least using an array might give you hash-like access times. Oh wait, I've already seen auto-generated code like this, in JavaScript:

myData[myData.length] = new Array("10","wtf","wheres my data?", "brillant!");

Of course, this was inside one <c:forEach> tag, so it did make some sense, as it wasn't actually backend code. But oh god, doing autogenerated code and using a switch() ??? Ouch!

Qwerty:

Self-modifying code was an exceedingly bad thing back in the 1980s when we had stand-alone micros like the TRS-80 and C-64. With a whole internet's worth of Nigerian scammers, it's an even worse idea to put user-input into your application's source code. What's the bet that this "internal order application" can be reached from outside? That's just opening up your wallet and saying "Take what you want."

 

 Hopefully the programmer would have thrown in a line to validate the data.  Then again, considering the WTF...

Hopefully the programmer would have thrown in a line to validate the data. Then again, considering the WTF...

Nope, it gets worse: $oid is a POST variable taken from a form, and there's no validation.

$oid = $_POST['idbox'];

redct:

Nope, it gets worse: $oid is a POST variable taken from a form, and there's no validation.

$oid = $_POST['idbox'];

 

Though to be fair 99% of people would try:

"; drop database;--,

how many people would try giving it:

";/* Arbitrary PHP code here */; $irrelevant="

It's clearly an awesome new form of protection against SQL injection...

fyjham:

Though to be fair 99% of people would try:

"; drop database;--,

Which doesn't work anyway...