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

[bug] refactor of IPC using URI schemes is breaking some usecase #7662

Closed
nikoPLP opened this issue Aug 21, 2023 · 11 comments
Closed

[bug] refactor of IPC using URI schemes is breaking some usecase #7662

nikoPLP opened this issue Aug 21, 2023 · 11 comments
Assignees
Labels
status: needs triage This issue needs to triage, applied to new issues type: bug

Comments

@nikoPLP
Copy link

nikoPLP commented Aug 21, 2023

Describe the bug

Nice to see an improvement on performances with the new IPC mechanism of #7170 .

It is obviously better to have data exchanged between the frontend and tauri, as body of POST requests instead of JSON in URLparam of a GET request.

But we shall note that this improvement is limited on platforms that are not android nor linux.

For me, it breaks a special use-case that I am having. I am loading a remote page (https) in a popup window from the tauri app, using the window API. In the popup window (not main) the javascript that is loaded contains the @tauri-apps/api and @tauri-apps/plugin-window modules, and makes use of the Window.getByLabel("main").emit() API call to send a message to the main window.

Until the breaking changes of 2.0.0-alpha.11, this was working fine, except on android, because of the way messages are passed on that platform.

Since the new IPC mechanism that uses ipc:// fetch, I get a security exception on the client side of the popup window.

No matter how much CSP header I injected on that page (via the HTTP server serving it), the error remains the same, as the page has an origin with https:// and ipc:// is considered insecure no matter what the CSP is saying.

So my https page cannot send messages back to the main tauri window anymore.

I guess this is a rare usecase, but I do make use of ipc_scope().configure_remote_access() API in the configuration of the tauri app, and I suppose that this API was meant specifically for this kind of use case, when a webpage making use of the client API is not served by tauri but by a remote server.

So it seems that the new IPC mechanism is breaking any possible usage of ipc_scope().configure_remote_access().

I first decided to freeze the tauri version i am using to 2.0.0-alpha.10 and to cherrypick some bug fixes that appeared in the latest version, and that I needed.

But then I thought this would not be future proof, so I decided to introduce a feature in tauri (in a fork of it, obviously) that would deactivate the new IPC mechanism and fall back to the old way using postMessage.

This crate feature that i named no-ipc-custom-protocol is quite minimalistic and seems to work on every platform i tested (all of them, except iOS that i don't have a dev env for, yet)

Would you consider introducing this feature in the next alpha release ?

Also I would really like if @lucasfernog, the author of the refactor, could find another way to circumvent this limitation and allow ipc:// to be fetched on any https:// page, but I am not aware of any solution for that. Another way might be to dynamically fall back to the older IPC mechanism when the calling webpage is from a remote origin, and let the new mechanism work on webpages that are served by tauri. That would allow the ipc_scope().configure_remote_access() to make sense again in the new IPC mechanism context.

The no-ipc-custom-protocol feature that I implemented can be found here :
https://git.nextgraph.org/NextGraph/tauri/commit/ec9b62b3379f5353c6d8b9484e12ec82ef4fa34f

Let me know if you want a merge request.

And thanks for your amazing work!

Reproduction

unfortunately I dont have enough time left for this issue to create a minimalist project that would reproduce this issue. Also, it would be a bit hard to setup as you need an external https server...
But if you have some work on a branch that you want me to test, I will do it happily.

Expected behavior

See description of bug

Platform and versions

[✔] Environment
    - OS: Mac OS 10.15.6 X64
    ✔ Xcode Command Line Tools: installed
    ✔ rustc: 1.69.0 (84c898d65 2023-04-16)
    ✔ Cargo: 1.69.0 (6e9a83356 2023-04-12)
    ✔ rustup: 1.26.0 (5af9b9484 2023-04-05)
    ✔ Rust toolchain: stable-x86_64-apple-darwin (environment override by RUSTUP_TOOLCHAIN)
    - node: 16.13.1
    - pnpm: 7.15.0
    - yarn: 1.22.18
    - npm: 8.1.2

[-] Packages
    - tauri [RUST]: (2.0.0-alpha.11)
    - tauri-build [RUST]: no manifest (2.0.0-alpha.6, 2.0.0-alpha.7)
    - wry [RUST]: 0.31.1
    - tao [RUST]: 0.22.2
    - @tauri-apps/api [NPM]: 2.0.0-alpha.6
    - @tauri-apps/cli [NPM]: 2.0.0-alpha.10

[-] App
    - build-type: bundle
    - CSP: unset
    - distDir: ../dist
    - devPath: http://localhost:1420/
    - framework: Svelte
    - bundler: Vite

Stack trace

No response

Additional context

No response

@nikoPLP nikoPLP added status: needs triage This issue needs to triage, applied to new issues type: bug labels Aug 21, 2023
@simonhyll
Copy link
Sponsor Contributor

Can you try the dev branch once #7677 has been merged in?

@nikoPLP
Copy link
Author

nikoPLP commented Aug 23, 2023

I can of course test that branch once the fix is merged, but after reading the bug report and the intended fix, I do not see this as related to my issue. I am having a security issue thrown by the browser because of mismatch origins. the browser doesnt allow ipc:// fetch in an https:// page. Even when adding CSP to the https page. As far as I understood the bug and fix you refer to, it is not the same issue.

Ok I understand you need a macos test of your fix, even though it is unrelated to my issue. Let me know once the merge is done on next, and i ll introduce an options argument in one of my commands to test it. That's fine!

And I understood everybody else is on vacation, so I ll wait for my own issue to be reviewed.

@lucasfernog
Copy link
Member

Seems like this only impacts macOS, WKWebView does some extended checks to prevent HTTPS to fetch a weird protocol :(
We'll fallback to the postMessage IPC in this case.

@nikoPLP
Copy link
Author

nikoPLP commented Sep 5, 2023

nice to see a fix already @lucasfernog . I will test it in the coming days.
2 comments :

  • if WKWebView is the problem, then you should probably exclude also ios target, right?
  • so if the new IPC mechanism doesn't work for linux, android, ios and mac, then... it only works for windows, right ?

@amrbashir
Copy link
Member

Not really, if you have a recent version of webkit2gtk installed (which is just a matter of time before it is available in all distros) and you enabled linux-ipc-protocol feature flag, and you're not using remote URLs on macOS and iOS, then you have windows, macos, ios and linux support.

@lucasfernog
Copy link
Member

I will open another PR to extend this for iOS and also fix the channel response usage.

The new IPC is used on all platforms, even if it's just to send bigger responses to speed up the process. It's just a little bit slower on Linux and Android (plus remote URLs on Apple) but it's still a lot faster than v1 IPC.

@nikoPLP
Copy link
Author

nikoPLP commented Sep 6, 2023

Thanks for the clarification !
I will test all of that once the new PR will be coded and merged. Thanks!

@nikoPLP
Copy link
Author

nikoPLP commented Sep 20, 2023

Hello @lucasfernog, any news about the new PR ?

@lucasfernog
Copy link
Member

@nikoPLP the fix has been released, can you try with latest alpha?

@nikoPLP
Copy link
Author

nikoPLP commented Sep 29, 2023

Yes all is good for me with the latest release (tauri alpha.14, api alpha.8)
I tested on all platforms, except iOS for which I don't have yet a dev env.
Thank you very much!

@gregpalaci
Copy link

I have this issue too, would be nice to just override and specify https:// protocol from the get go, very confusing errors. Will try no-ipc-custom-protocol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs triage This issue needs to triage, applied to new issues type: bug
Projects
None yet
Development

No branches or pull requests

5 participants