nativescript-community / https

Secure HTTP client with SSL pinning for Nativescript - iOS/Android
https://nativescript-community.github.io/https/
Other
50 stars 42 forks source link

HTTPS response is different on iOS and Android #26

Open exAbstracto opened 6 years ago

exAbstracto commented 6 years ago

We're implementing a rest server and a mobile client to call the various rest endpoints (via https, with nativescript-https). On Android, the response is a plain JSON, ready to be used (accessing the fields can be done with response.field1, response.field2 etc.) On iOS, the response is a NSDictionary object, getting the actual data from it has to be done with response.valueForKey("field1"), response.valueForKey("field2") etc. This is not optimal, it brings a platform dependency that we would like to avoid. We have rest calls througout the application, thus a lot of potentially platform dependent points.

CLI:  3.4.0
Cross-platform modules: 3.4.0
Runtime(s): android 3.4.0 and ios 3.4.0

var https = require("nativescript-https");

function someHttpsCall = function() { return https.request({ url: "serverURL/restEndPoint", method: "GET", headers: { "Content-type": "application/json", }, }).then(handleErrors) // implemented somewhere else .then(function(response) { return response.content.data; }) .then(function(data) { // ------> Here, data is a JSON in Android, and a NSDictionary in iOS } }

exAbstracto commented 6 years ago

Hi @roblav96 any chance to look at this issue? Thank you!

rhclayto commented 5 years ago

I'm no Objective-C coder, & I don't have an iOS device to test on, but I see in the function AFSuccess(resolve, task: NSURLSessionDataTask, data: NSDictionary<string, any> & NSData & NSArray<any>) { function block:

if (data && data.class) {
        // console.log('data.class().name', data.class().name)
        if (data.enumerateKeysAndObjectsUsingBlock || data.class().name == 'NSArray') {
            // content = {}
            // data.enumerateKeysAndObjectsUsingBlock(function(k, v) {
            //  console.log('v.description', v.description)
            //  content[k] = v
            // })
            let serial = NSJSONSerialization.dataWithJSONObjectOptionsError(data, NSJSONWritingOptions.PrettyPrinted)
            content = NSString.alloc().initWithDataEncoding(serial, NSUTF8StringEncoding).toString()
            // console.log('content', content)
        } else if (data.class().name == 'NSData') {
            content = NSString.alloc().initWithDataEncoding(data, NSASCIIStringEncoding).toString()
            // } else if (data.class().name == 'NSArray') {
            //  content = []
            //  let i: number, len: number = data.count
            //  for (i = 0; i < len; i++) {
            //      let item
            //      let result: NSDictionary<string, any> = data[i]
            //      if (result.enumerateKeysAndObjectsUsingBlock) {
            //          item = {}
            //          result.enumerateKeysAndObjectsUsingBlock(function(k, v) {
            //              item[k] = v
            //          })
            //      } else {
            //          item = data[i]
            //      }
            //      content.push(item)
            //  }
        } else {
            content = data
        }

        try {
            content = JSON.parse(content)
        } catch (e) { }

    } else {
        content = data
}

The first thing I notice is that there is a lot of commented code; it looks like there was some trial & error going on.

Anyway, the data parameter is going to be an NSDictionary object, which in the code should be turned into JSON. However, the problem might be that it happens inside this check, if (data && data.class) {. From looking at some documentation about the NSDictionary object, it doesn't look like there is a class property, but there is a class() method on it. So this evaluates to false, & the code that makes the NSDictionary into JSON is skipped, & the NSDictionary is returned as-is.

I believe the check should be if (data && data.class()) {.

rhclayto commented 5 years ago

I've created a pull request with this change. Like I said, I haven't tested this myself. https://github.com/gethuman/nativescript-https/pull/40