REST and XML are hard...

RandyHickey

So this year I inherited an old (as in 3 years) PHP project for maintaining it. Being built on Tigermouse ("Tigermouse is a modern web applications framework for PHP. It relies on AJAX technology, so in pair with being lightweight enables you to build dynamic, responsive and interactive web applications."), it's the worst piece of shit code I've seen in my career. I will probably need to post quite a few topics about it, because putting all the awfullness in one topic would be too much.

So, let's jump right in, shall we? Our primary WTF today: REST and XML are hard..., just to give you an example of the general programming attitude that was in place before we got the project, even though it has relatively little that can be blamed on Tigermouse here.

This project of ours has a "REST" API to manage a few, relatively simple database entities (of the many there are). Nevermind the fact that to authenticate API requests, you have to send your username and password in cleartext as query string parameters (/api/something?user=...&password=...). Nevermind that there's no SSL anywhere. Nevermind that it's so far away from REST that Roy Fielding would probably start to cry if he ever saw it. Nevermind the fact that it manually constructs XML strings without properly escaping data supplied by unauthenticated users on the Internet. Nevermind that the API is implemented directly inside the database model classes (MyDatabaseModel::dispatchPostRequest($request)).

But the cake won another aspect of this monster: The code is trying to convert an XML document to an array. To accomplish this, the original developer created an XmlParser class to convert the XML to an array structure. Given the following XML

<?xml version="1.0"?>
<root>
  <child/>
</root>

the parser would create this (printed in JSON because it's shorter than print_r()):

{
  "ROOT": {
    "ATTRIBUTES": null,
    "VALUE": {
      "CHILD": {
        "ATTRIBUTES": null,
        "VALUE": null
      }
    }
  }
}

But if you then add a <child> tag

<?xml version="1.0"?>
<root>
  <child/>
  <child/>
</root>

to your document, the structure changes to

{
  "ROOT": {
    "ATTRIBUTES": null,
    "VALUE": {
      "CHILD": [
        {
          "ATTRIBUTES": null,
          "VALUE": null
        },
        {
          "ATTRIBUTES": null,
          "VALUE": null
        }
      ]
    }
  }
}

So CHILD turned into an array of nodes. Fixing this (i.e. creating a stable structure for the surrounding code) was hard apparently, judging from the mountains of debugging statements left behind:

//  debug(print_r($this->data,1)); // CORRECT is full of data
// debug(print_r($ds,1)); // CORRECT is full of data
$parser = xml_parser_create($charset);
xml_parser_set_option($parser, XML_OPTION_SKIP_WHITE, 1);
xml_parser_set_option($parser, XML_OPTION_TARGET_ENCODING, "UTF-8");
$vals = array ();
$index = array ();
xml_parse_into_struct($parser, $this->data, $vals, $index);
xml_parser_free($parser);
// debug(print_r($this->data,1)); // CORRECT is full of data
// debug(print_r($vals,1)); // CORRECT is full of data
$return = $this->getChildren($vals, 1);
 //debug(print_r($return,1)); // EEEEEMPPPTTYYYY

// ...snip ...

/*
$tagtwo = "FIELD";
$rtwo[] = array ( "ATTRIBUTES" => "EMAIL",  "VALUE" => "hahah@jaddajadda.net" );
debug(print_r($rtwo,1)); // empty... of course....
*/
// debug(print_r($r,1)); // empty... of course....

So instead of creating something proper or just abandon the idea alltogether and stick with using the XML tree, you find this little nugget in ~40 spots in the codebase:

if ($tree['ROOT']['VALUE']['SOMEELEMENT']['VALUE']['ANOTHERELEMENT']['ATTRIBUTES']) {
  $tree['ROOT']['VALUE']['SOMEELEMENT']['VALUE']['ANOTHERELEMENT'] = array (
    $tree['ROOT']['VALUE']['SOMEELEMENT']['VALUE']['ANOTHERELEMENT']
  );
}

But even fixing the codebase like this was too hard, because the original developer simply forgot or was too lazy to fix the dispatchXXXRequest() methods in the same way. Making the surrounding code puke and throw Exceptions.

Instead, they extended the API documentation to say:

If you are sure that you are sending valid XML but still receive HTTP 500 Internal Server Errors, please include dummy elements like <someelement dummy="true" /> and try again.

Problem solved.

aapis

Broken by design. Love it.

blakeyrat

@RandyHickey said:

("Tigermouse is a modern web applications framework for PHP. It relies on AJAX technology, so in pair with being lightweight enables you to build dynamic, responsive and interactive web applications.")

Wow, I thought that was a transcription error, but you actually copied and pasted that from their About page. Huh.

If anybody wants to volunteer on a open source project, these guys need a copy editor STAT!

@RandyHickey said:

Nevermind the fact that to authenticate API requests, you have to send your username and password in cleartext as query string parameters (/api/something?user=...&password=...).

Haha oh wow.

@RandyHickey said:

Fixing this (i.e. creating a stable structure for the surrounding code) was hard apparently, judging from the mountains of debugging statements left behind:

This is where a normal programmer might back-up and question their approach instead of just charging right ahead.

Keith

@blakeyrat said:

This is where a normal programmer might back-up and question their approach instead of just charging right ahead.

You could almost suggest that these people should be excluded from participation in software development...

HardwareGeek

I see what you did there. And I approve.

ben_lubar

@RandyHickey said:

if ($tree['ROOT']['VALUE']['SOMEELEMENT']['VALUE']['ANOTHERELEMENT']['ATTRIBUTES']) {
$tree['ROOT']['VALUE']['SOMEELEMENT']['VALUE']['ANOTHERELEMENT'] = array (
$tree['ROOT']['VALUE']['SOMEELEMENT']['VALUE']['ANOTHERELEMENT']
);
}

Protip:

$tree['ROOT']['VALUE']['SOMEELEMENT']['VALUE']['ANOTHERELEMENT'] = (array) $tree['ROOT']['VALUE']['SOMEELEMENT']['VALUE']['ANOTHERELEMENT'];

cartman82

Oh that makes it better. Not a WTF anymore.

algorythmics

I wonder if @SignatureGuy works elsewhere? he belongs in all sidebar threads

Arantor

See, they're already fucked from the off by using one of the more retarded XML library bundlings for PHP.

I know it's already retarded that PHP has several XML things available for it, and all of them ultimately use libxml2 in the first place but FFS SimpleXML exists for a reason and is almost certainly guaranteed to be less fucked up than the code above.

Then again the fact they're not using it is probably demonstrative that if they did, they'd still fuck it up anyway.

dkf

@RandyHickey said:

Nevermind the fact that to authenticate API requests, you have to send your username and password in cleartext as query string parameters (/api/something?user=...&password=...). Nevermind that there's no SSL anywhere.

*twitch* *twitch* *twitch*

Arantor

I think that's TRWTF of the entry, sorry I missed it on frist reading.

abarker

@RandyHickey said:

Nevermind the fact that to authenticate API requests, you have to send your username and password in cleartext as query string parameters (/api/something?user=...&password=...). Nevermind that there's no SSL anywhere.

BURN IT WITH FIRE!!!!

Polygeekery

@RandyHickey said:

Tigermouse

The most recent update to their project website is in 2008. You are screwed.

ben_lubar

@LikeBot

RandyHickey

Even if there was some development still going on, I'd doubt that the original developer would do the necessary things (i.e. throw everything away, apologize and start over). ;-)

dkf

The necessary thing does involve 17 long nails, 5 angry hippos and a large box of toffee apples.

SignatureGuy

I agree with whatever @algorythmics posted just above.<t3075p8>

LikeBot

Filling in for SignatureGuy, I agree with whatever @ben_lubar posted just above.

algorythmics

wow, sweet 12 hour delay!