[TIMOB-2480] iOS: httpclient assumes XML with reponseText emitting parse error

Note that most recent tests using Dec 1 2010 18:59 r44a760a9 nightly

According to another developer on Q&A, the last working httpclient in 1.5.0 without parsing error has githash of 3ee6a97 who has another simple use case to demonstrate issue at http://developer.appcelerator.com/question/84881/httpclient-broken-for-150--githash1dd0106"> http://developer.appcelerator.com/question/84881/httpclient-broken-....

Looks like Steven Tramer is to blame for issue in result proxy. There were a few commits Nov 9, 10 after which Jeff Hanie attempted a fix of this very issue on Nov 22 so it was recognized at that time but fix is apparently not adequate. Jeff's fix was to examine Content-Type in header to differentiate however I am able to verify that Content-Type in response is application/json.

       @try {
           value = [delegate valueForKey:key];
           if ([key isEqual:@"responseXML"])
           {
               // check response content-type before trying to parse into XML - gets rid
               // of the silent XML parse error when not XML content
               id ct = [delegate getResponseHeader:[NSArray arrayWithObject:@"Content-Type"]];
               if ([ct rangeOfString:@"xml"].location==NSNotFound)
               {
                   return;
               }
           }
       }
   

sending for triage

Odd. That code checks for "responseXML" which MUST be valid XML, not 'responseText', which can be any old text. What is the sample code for this? To get a proper fix, it'd be necessary to test against the actual server, or at very least, the expected result from the server as to work with the real data.

Blain. Exactly. I have sent Thomas complete sample apps and in addition to link for couchDBX that you can use to test. No xml there, JSON request and responses only. The code and server tests that I spend a good amount of time detailing for you can all be run within 15 mins, Please let me know if you would like me to send it directly to you. Need your email address to do so.

Blain. The info I sent Thomas also contains curl comparisons made against the server. This was one of a few issues I brought forward concerning httpclient including getResponseHeaders that was recently fixed. There are others outstanding including this one, which is the most serious that is a show stopper for any web services work with JSON. I am using pre Nov 9 httpclient files with 1.5.0 nightly build in order to continue my own development without the issue.

I'm the author of the post http://developer.appcelerator.com/question/84881/httpclient-broken-for-150--githash1dd0106"> http://developer.appcelerator.com/question/84881/httpclient-broken-...

The last commit that suppress the responseXML parse errors still gives error in some cases:

I simply call a link that makes a redirect with a 302, and try to get the new location and nothing more (so no parsing for any kind of response).

The headers the server sends seems to be ok to me, but the location never gets populated.

The older commit githash=3ee6a97 works just fine.

Here are the headers:

Tamas-Daniels-MacBook-Pro:~ Spawn$ curl -vvv http://www.motorsport-total.com/f1/live/htdocs/ticker.php">http://www.motorsport-total.com/f1/live/htdocs/ticker.php
About to connect() to http://www.motorsport-total.com">www.motorsport-total.com port 80 (#0) Trying 87.119.204.100... connected * Connected to http://www.motorsport-total.com">www.motorsport-total.com (87.119.204.100) port 80 (#0)

GET /f1/live/htdocs/ticker.php HTTP/1.1 User-Agent: curl/7.19.7 (universal-apple-darwin10.0) libcurl/7.19.7 OpenSSL/0.9.8l zlib/1.2.3 Host: http://www.motorsport-total.com">www.motorsport-total.com Accept: /

< HTTP/1.1 302 Found
< Date: Wed, 08 Dec 2010 21:01:02 GMT
< Server: Apache/2.0.52 (CentOS)
< Set-Cookie: f1totaluser=q67sgsc5fcouj6vp9j47d1bn80; path=/
< Expires: Thu, 19 Nov 1981 08:52:00 GMT
< Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0
< Pragma: no-cache
< Location: http://www.motorsport-total.com/f1/live/htdocs/ticker.php?strecken_id=19&;event_id=7&kunde=default"> http://www.motorsport-total.com/f1/live/htdocs/ticker.php?strecken_...
< Content-Length: 0
< Connection: close
< Content-Type: text/html; charset=iso-8859-1
<
* Closing connection #0

I think I found the issue
in TiNetworkHTTPClientProxy.m you are releasing to nil the request after it finishes.

   
   -(void)requestFinished:(ASIHTTPRequest *)request_
   {
       [self _fireReadyStateChange:NetworkClientStateDone];
       if (connected)
       {
           connected = NO;
           [[TiApp app] stopNetwork];
       }
       RELEASE_TO_NIL(request);
   }
   

I commented the RELEASE_TO_NIL(request); line and location is back in place

Blain - This issue was repaired two days ago by Steven Tramer who fixed his original work that caused the problem. Why has this ticket not been updated?

Noting for others that response codes have recently been changed to text from integers.

I believe this issue ought to have been categorized as a high priority. The client is such an important component to the SDK and when issues like this arise, it is impossible to carry on productive development (as opposed to a UI bug that you can afford to work around or wait until fixed).

Still missing from the ticket is any sample code to fully recreate the issue and more importantly, verify any impact beyond a false warning that is safely contained within a try/catch structure.

Is there any example JS to better illustrate the impact beyond the spurious warning? This may be like the 4.1 issue where there was a warning about missing symbols that did not have any actual impact on performance.

Blain. Comprehensive documentation of the issue including three sample apps had been sent to Thomas Huelbert almost two weeks ago. He advised he would ensure it would be forwarded to whomever was assigned the issue. This is noted in the first paragraph when ticket was filed. I spent significant time writing this up. This was followed up with detailed information with what was broken, when it occurred and who was to blame. Please see Thomas for the info which was emailed to him Dec 1.

Seeing this sort of request for additional information almost two weeks afterwards does not illicit good feelings since you are late to the party. Steven Tramer modified the code about 4 days ago. Issue cannot be duplicated since the issue has since been resolved.

This was a high priority item with a release immanent. Thomas received the information but was not responsible for the ticket. Steven did the work (possibly with or without the information) and almost two weeks later, you want more information after it has been fixed with notations in the repository. So far as I am concerned, I would have expected Steven to communicate on the ticket. For most projects, the repository and tickets system are the primary means of developers communication and not stepping on each other's toes. I sincerely want to help you folks and possibly contribute bug fixes in future however this sort of experience can't help.

Blain,
Here is the issue with the location - build 1.5.1 9819ce0 13/12/2010

    
    var xhr = Titanium.Network.createHTTPClient();
    
    xhr.onload = function(e){
        Ti.API.info('onload');
        Ti.API.info(xhr.location);
        // Ti.API.info(xhr.responseText);
    };
    
    xhr.onerror = function(e){
        Ti.API.info('onerror');
        // Ti.API.info(xhr.responseText);
    };
    
    xhr.open('GET','http://www.motorsport-total.com/f1/live/htdocs/ticker.php');
    xhr.send();
    

The result is:

    
    [INFO] test/1.0 (1.5.1_9819ce0_13122010.9819ce0)
    [INFO] onload
    [INFO] <null>
    

Commenting the

    
    RELEASE_TO_NIL(request);
    

line in TiNetworkHTTPClientProxy.m in function -(void)requestFinished:(ASIHTTPRequest *)request_

puts the location back in place

    
    [INFO] test/1.0 (1.5.1_9819ce0_13122010.9819ce0)
    [INFO] onload
    [INFO] http://www.motorsport-total.com/f1/live/htdocs/ticker.php?strecken_id=19&;event_id=7&kunde=default
    

Tamas, I think the issue with xhr.responseText is something else. I've opened it as #2573. As mentioned on the bug, this is an known compromise to stop memory issues caused by recursive XHR chains, and the workaround is to use 'this'.

This bug may be fixed by #2450 and #2573. Please test and let us know if this is the case.

Have tested from 1.5.1 continuous build and changes appear good. There are two additional issues I have identified with iOS httpclient but will file under separate tickets. So far as I am concerned this ticket should be marked resolved.

Going to fixed-in-qa this due to lack of activity and the fact that it seems to be a dupe of several bugs.

[INFO] Titanium SDK version: 1.6.0 (01/13/11 08:11 7ca73a3)