MGTwitterLibXMLParser incorrectly handles creating dictionary in userDictionaryForNodeWithName #61

Open
adamvduke opened this Issue Oct 21, 2010 · 1 comment

Comments

Projects
None yet
1 participant

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
<screen_name>snakes_nbarrels</screen_name>
<created_at>Wed Sep 29 01:01:54 +0000 2010</created_at>
......

<created_at>Wed Oct 20 21:34:30 +0000 2010</created_at>
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 <created_at> 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".

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment