mattgemmell / MGTwitterEngine

Objective-C Twitter integration library for Mac OS X and iPhone. Official repository.
http://mattgemmell.com/source
1.13k stars 164 forks source link

MGTwitterLibXMLParser incorrectly handles creating dictionary in userDictionaryForNodeWithName #61

Open adamvduke opened 14 years ago

adamvduke commented 14 years ago

First the URL I'm getting the data from http://twitter.com/users/show/196397766.xml

The data looks something like the following:

196397766 The Engineer snakes_nbarrels Wed Sep 29 01:01:54 +0000 2010 ...... Wed Oct 20 21:34:30 +0000 2010 27966251872 ........

The resulting dictionary, with the same irrelevant data removed is below: { "created_at" = "Wed Sep 29 01:01:54 +0000 2010"; id = 2147483647; name = "The Engineer"; "screen_name" = "snakes_nbarrels"; status = "Wed Oct 20 21:34:30 +0000 2010"; }

I stepped through the code and the 'id' value is originally written correctly, but over-written during the processing of the status sub-tree of the xml. The difference in value is explained by Issue #54. There is an attempt to get the intValue of the string 27966251872, which just ends up being NSIntegerMax.

The other interesting piece is that the node under has had it's value put in the dictionary with the key 'status'. After looking at the code it seems to me that the problem is with the -(xmlChar *)_nodeValue method. The method is not written to handle the case where the current node has children, rather than text. If the reader is advanced to the node before the call to _nodeValue, the method advances the reader because the xmlReaderType is not XML_READER_TYPE_TEXT. During the next iteration it does find the correct reader type, but the caller is expecting a value for the parent node. I would think that the _nodeValue method should return the entire node, in this case

<status>......</status>
and leave the decision making about what to do up to the caller.

I worked with it a bit, but I'm not familiar enough with the libxml API to get it to do the right thing. I was able to come up with a workaround using a similar pattern to what is used in the statusDictionaryForNodeWithName method. I added an else if clause to check if the current node is "status" and if so add a dictionary from statusDictionaryForNodeWithName to the current dictionary with the key "status".

adamvduke commented 14 years ago

I forked and made two separate commits. One for the workaround and one to add a //TODO to fix the behavior of nodeValue, or at least point out that something might not be exactly correct with it.