Skip to content
This repository has been archived by the owner on Nov 7, 2018. It is now read-only.

add options to disable local web server, implement local proxy to handle server no cors support #115

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JiaLiPassion
Copy link

@JiaLiPassion JiaLiPassion commented Mar 26, 2017

Platforms affected

ios

What does this PR do?

fix #124

  1. add an option in config.xml to disable local web server.

  2. add an option in config.xml to start a local proxy server instead of local web server, that means the local files(index.html/js/css) are still loaded locally, and the xhr request will go through
    the local proxy server and use native http to communicate with remote server.
    so server will not need to set CORS Allow-Origin to *.

  3. add logic to prevent the port 8080 is used, when 8080 is used, try 8180, 8280..8580.

What testing has been done on this change?

I do some basic test on my own ionic project.

Details

  1. add an option in config.xml to disable local web server.

ProxyMode:

  • LOCALWEB (default)
    the same with wk3.0, use localhost:8080

  • LOCALPROXY
    static files will not go through GCDWebserver, only POST xhr request go through the GCDWebserver and redirect with AFNetworking request. so server will not need to handle CORS issue. because some server we can't control, so we can't let then open the permissions.

in this mode, the request sequence will look like.

  - application request : http://remoteurl/api
  - xhr intercept and redirect to: http://localhost:8080/api, 
and set the http://remoteurl/api to header['X-original-url']
  - GCDWebServer act as proxy and get the http://remoteurl/api
 from header then send the request with AFNetworking
  - get the response and return the application 
  • DISABLEPROXY
    will not start local web server or proxy server.

2.add an option in config.xml to start a local proxy server instead of local web server, that means the local files(index.html/js/css) are still loaded locally, and the xhr request will go through
the local proxy server and use native http to communicate with remote server.

based on options, if it is LOCALPROXY

  • inject wk-xhr.js to monkey-patch XMLHttpRequest, and
  - set the url to local proxy url
 - keep the original url to header with 'X-original-url'
  • Start GCDWebserver and accept POST request, get the original url from 'X-origin-url'
    then use AFNetworking to send request and redirect the response to user.
  1. add secret(UUID) in header, so user can't access the local web server / local proxy server
    from other application.

@manucorporat , I will continue to test the PR, could you help to check whether this PR is reasonable or not?

The motivation of the PR is

  • Server side don't need to open permission of CORS(because some server we can't control)
  • Local server port conflict issue

Thank you so much!

@JiaLiPassion JiaLiPassion force-pushed the proxyoption branch 4 times, most recently from 4e0d188 to 09d3968 Compare March 27, 2017 05:41
@JiaLiPassion JiaLiPassion changed the title add options to disable local web server, implement local proxy add options to disable local web server, implement local proxy to handle server no cors support Mar 30, 2017
@virtualprodigy
Copy link

Will this fix be added? CORS for XHR request is a major issue with this framework.

@JiaLiPassion
Copy link
Author

@manucorporat, could you check that this idea is ok or not?

@manucorporat
Copy link

@JiaLiPassion thanks for the PR, but this is very unlikely to be merge.

  1. I tries to fix too many things at once
  2. It adds a big dependency for what?
  3. The change is not a clean one, it introduces tons of trailing whitespaces.

Also, how are you having a port conflict? you should open an issue

@JiaLiPassion
Copy link
Author

@manucorporat , thank you for your reply.

And the current PR is just an working demo, and just like you said, it have a lot of conflicts, whitespaces and other issues.

  • I want to discuss with you that the idea is ok or not? Because CORS is a main issue in my projects.
    And I don't want to let application code use native http directly.

  • And about the port conflict, I think the GCDWebServer is a global one, and if two application use cordova-plugin-wkwebview, there will have port conflict, and I will open an issue later.

Thanks again and could you check the idea is ok or not?
If this is worth discussing, I will try to make this PR better!

@manucorporat
Copy link

iOS apps are sandboxed by apple (on device). Each app has his own "localhost", there are not conflicts having several apps open the same port, because they are sandboxed.

Have you taken a look at this:
https://ionicframework.com/docs/native/http/
?

@JiaLiPassion
Copy link
Author

JiaLiPassion commented May 21, 2017 via email

@emiraydin
Copy link

@manucorporat This issue renders this plugin useless for anyone using Ionic v1 (assuming your app will make HTTP requests and turning CORS on server-side is not a good security practice). It would be great to respond to the solution @JiaLiPassion is proposing at the idea level, because I agree the PR looks dirty but the problem it is addressing is real and an important one.

@manucorporat
Copy link

@emiraydin can you explain why it is a security risk?

@manucorporat
Copy link

There are very good reasons of why this plugin works the way it does

@Alexintosh
Copy link

@manucorporat Can you tell us more about this plugin works the way it does? I'll be interested in understanding more.

@Bessonov
Copy link

Maybe relevant for #101 and #163.

@JiaLiPassion can you give me an example how to switch to LOCALPROXY? Can you please rebase against the master?

@JiaLiPassion
Copy link
Author

@Bessonov , yeah, I will rebase and redo this PR this week.

@Bessonov
Copy link

@JiaLiPassion great! it would be nice if path to api can be configured too, because we use /service instead of /api. Can you tell my how I can configure your version to use LOCALPROXY? So I can try to test your current implementation.

@JiaLiPassion
Copy link
Author

@Bessonov

you can add

<preference name="ProxyMode" value="LOCALPROXY"/>

in config.xml.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wkwebview will fail if server not allow cors.
6 participants