nativescript-community / https

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

Body is not set on GET method #59

Open panagulis72 opened 4 years ago

panagulis72 commented 4 years ago

Hi, I'm making a GET call but no params/body are send to the server:

let httpRequestHeader = { Content-type:"application/json" };
let httpRequestBody = { test: '123' };
Https.request({
                url: endPoint,
                method: 'GET',
                timeout: 30,
                body: httpRequestBody,
                params: httpRequestBody,
                headers: httpRequestHeader
            }

the header is set correctly, but the body of the request are ignored, because; no data reaches the server. I'm using android with the last version of nativescript-https

panagulis72 commented 4 years ago

I tried to append params (....?param2=xxx&param3=yyyy"") to the url (endPoint), and it works. I think it would be better to handle "params" (Https.HttpsRequestObject) in "GET" requests, so we don't have to pass all the parameters manually from the url

kefahB commented 4 years ago

Your endpoint point to localhost ? if you try to reach localhost on simulator you must declare your endpoint to http://10.0.3.2

kefahB commented 4 years ago

For the query params I use buildurl

content = {
   foo: "bar",
   bar: "foo"
};
let body    = "";
        if (content && method == 'GET')
        {
            let path = "https://domaine.com";
            url = buildUrl(this.end_point, {
                path: "/" + this.api_vesion + path,
                hash: '',
                queryParams: content
            });
            body = "";
        }
vaab commented 3 years ago

Hi, I hit the same issue. This is my take on it :

the android implementation will not build and send any body for HEADER and GET requests. This probably stems out the fact that servers should probably ignore any body in these requests. See the discussion here to understand the context:

https://stackoverflow.com/questions/978061/http-get-with-request-body

Please understand that I'm talking about real body of the HTTP request, and not the querystring way to send data.

In the current code: https://github.com/nativescript-community/https/blob/master/src/https.android.ts#L417-L418 we can see that the data provided in opts, in opts.body or opts.content, will not be used to create a body to the actual request sent by this module in the android implementation. I had a quick look in the ios implem, and it doesn't seem to have this drawback.

I would strongly suggest to build the body (from opts.body if existing), and think about using querystrings to send opts.content... but I have a case here were we are expecting opts.content to be in JSON in the body.

Some important services (ElasticSearch is mentionned in the first link) rely on this even if it should not be used.

We can create such request with no problem with curl for instance.

For clarity, this is a full HTTP request with a body:

GET /foo HTTP/1.1
Host: bar:8888
Accept: */*
Content-Type: application/json
Connection: close
Content-Length: 14

{"foo": "bar"}

I can provide a fix for the issue if necessary, but would welcome any suggestion about what would be the best to do around querystring data and contents of both opts.body and opts.content.

farfromrefug commented 3 years ago

@vaab you can try to fix this but i dont think okhttp supports it. Having a body in a GET request is not really "in the specs". You should use a POST method in this case. One reason is that i think it would break the cache feature.