Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for proxy cross domain problem #75

Merged
merged 1 commit into from Jul 21, 2014
Merged

Fix for proxy cross domain problem #75

merged 1 commit into from Jul 21, 2014

Conversation

MrBr
Copy link
Contributor

@MrBr MrBr commented Jul 20, 2014

Headers wasn't forwarded properly as I could see, and for some reason if host is in header it breaks.
Also, body needs to be ether string or array of strings (not sure for array values).

There is maybe better place to do this fix but I am not nodejs dev so it takes me time to connect everything.

My suggestion, do not place host into headers and leave body in format that is in ajax. I am not sure where this modifications happen, maybe is bug in some other lib.

Headers wasn't forwarded properly as I could see, and for some reason if host is in header it breaks.
Also, body needs to be ether string or array of strings (not sure for array values).

There is maybe better place to do this fix but I am not nodejs dev so it takes me time to connect everything.

My suggestion, do not place host into headers and leave body in format that is in ajax. I am not sure where this modifications happen, maybe is bug in some other lib.
mwbrooks added a commit that referenced this pull request Jul 21, 2014
[#75] Fix for proxy cross domain problem
@mwbrooks mwbrooks merged commit 544867b into phonegap:master Jul 21, 2014
mwbrooks added a commit that referenced this pull request Jul 21, 2014
@mwbrooks
Copy link
Collaborator

Hi @MrBr,

Thanks for putting together this pull request! The body mapping was the best solution that I had found as well. Good catch on deleting the host header (not sure why it happens either).

I think this solution is better than nothing, so let's kick it into the wild and see whether it helps some of the users with their POST request issues.

@mwbrooks
Copy link
Collaborator

Published as:

connect-phonegap@0.12.3
phonegap-cli@3.5.0-0.20.7

@MrBr
Copy link
Contributor Author

MrBr commented Jul 21, 2014

Hi @mwbrooks ,

Glad I contributed.
Also excited to see how will it go with others, following feedback.

@seme1
Copy link

seme1 commented Jul 22, 2014

I've update phoonegap to 3.5.0-0.20.7 and also updated the phonegap developer app on my iphone. Unfortunately, the changes did not have any effect and the ajax post requests are not going through (still showing as "Pending" with Weinre)

@MrBr
Copy link
Contributor Author

MrBr commented Jul 22, 2014

Thanks to your comment I have done few more test, and noticed that it ain't working if data is type of object, but only as string. @seme1 can you confirm that you have done ajax request with data type of object ?

So, I've done few more tests and come with fix for that. Made pull request, and now we'll have to wait what Brooks says.

@seme1
Copy link

seme1 commented Jul 22, 2014

Hi

This is how my ajax requests now look like:

var dataToSend =[];
dataToSend['commentText']="Sometext";
$.ajax({ method: 'POST', url: domainURL ,processData: true, data:dataToSend,dataType: "json",
           success: function (results) {
             console.log("success");
},error:function(data){
             console.log("fail");
});

The above is not working. I simply updated phonegap with the command:

npm update -g phonegap

I now have version 3.5.0-0.20.7 installed in my system and the above code is still not working.

@MrBr: if I knew how to test the changes you made on my system, I would do it right away. If it is simple enough and you can guide me on how to do, it'd be very much appreciated. Otherwise, we can wait for @mwbrooks to push the changes you made so that the above "update -g phonegap" command reflects the new changes.

@MrBr
Copy link
Contributor Author

MrBr commented Jul 22, 2014

@seme1 I have done again some test, and this is result:
You can't send data type of array, it must be object or string, which means

var dataToSend =[];
dataToSend['commentText']="Sometext";

Should be

var dataToSend ={};
dataToSend['commentText']="Sometext"; //this can stay but you can also write with "."
dataToSend.commentText="Sometext"; //better way for objects

However, even if you send it as object at this point it won't work for now, I have made pull request which has to be merged and then it will work.

Working way to send POST data at this point is to send it as string

var dataToSend='commentText=sometext&otherVar=otherVal';

NOTE, as I say, with this new pull you should be able to send it as object but you have to wait bit more.

@mwbrooks
Copy link
Collaborator

Thanks Luka and @seme1 for working through this together! I'm on the road
right now but @timkim is holding down the repos with publisher rights.
Hopefully we can resolve this proxy headache for everyone!

Cheers,
Michael

On Tuesday, July 22, 2014, Luka Bracanović notifications@github.com wrote:

@seme1 https://github.com/seme1 I have done again some test, and this
is result:
You can't send data type of array, it must be object or string, which means

var dataToSend =[];
dataToSend['commentText']="Sometext";

Should be

var dataToSend ={};
dataToSend['commentText']="Sometext"; //this can stay but you can also
write with "."
dataToSend.commentText="Sometext"; //better way for objects

However, even if you send it as object at this point it won't work for
now, I have made pull request which has to be merged and then it will work.

Working way to send POST data at this point is to send it as string

var dataToSend='commentText=sometext&otherVar=otherVal';

NOTE, as I say, with this new pull you should be able to send it as object
but you have to wait bit more.


Reply to this email directly or view it on GitHub
#75 (comment)
.

@MrBr
Copy link
Contributor Author

MrBr commented Jul 22, 2014

I also hope this will work completely now, and I am very glad I contributed to PhoneGap!

mwbrooks added a commit that referenced this pull request Jan 7, 2015
[#175] [#75] Use node-http-proxy to proxy and rewrite headers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants