Skip to content

Support cross-domain in both directions with single proxy page#30

Closed
gerneio wants to merge 1 commit intojcubic:masterfrom
gerneio:patch-1
Closed

Support cross-domain in both directions with single proxy page#30
gerneio wants to merge 1 commit intojcubic:masterfrom
gerneio:patch-1

Conversation

@gerneio
Copy link

@gerneio gerneio commented Feb 13, 2022

Pass proxy invoke calls BACK to parent frame with postMessage. Tested for my individual purposes (broadcast, post), so I'd be surprised if any of the events are showing accurately. Really the proxy page needs to point to it's own abstracted JS file that forwards along each request/event correctly, with out all the logic bits (i.e. a router).

Ref #29

@gerneio gerneio marked this pull request as ready for review February 13, 2022 03:27
@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

Can you show me a live demo when this doesn't work and your code does? I'm not sure if I understand the use case.

Usually, when I want two-direction communication I used two iframes (and this is how it's documented) if it will work with a single iframe that would be great and it will simplify the setup (also the documentation - README - will need to be updated).

@gerneio
Copy link
Author

gerneio commented Feb 13, 2022

@jcubic So using your demos sites (https://jcubic.pl/sysend.php & https://terminal.jcubic.pl/sysend.php) to test the issue, just remove the proxy calls from one of the pages. To test, I used Chrome dev tools local overrides features and modified "https://jcubic.pl/sysend.php" so that the proxy lines were commented out (see attached PHP file: sysend.zip). I then opened both pages simultaneously and could send messages from "jcubic.pl" to "terminal.jcubic.pl", but nothing would be received if sent in reverse.

I then did the local override for the "sysend.js" file to point to my modified version of the script, refreshed the pages, and then the messaging worked from either direction. Therefore it is possible to have only a single proxy in use.

Just so you know, my use case is where I have full control over one web server, however the other I am only allowed a custom loaded JS file, but can't host any sort of proxy page. But the custom JS file is really all I need since I have full control of the other server/domain.

@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

Will this work when the iframe is on either of the domains?

@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

I Will test this and release it as the next feature version. Since if only one domain will need an iframe it would be a huge change.

@gerneio
Copy link
Author

gerneio commented Feb 13, 2022 via email

@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

I've tested and it doesn't work 100% correctly. The events are duplicated.

@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

This is what I have after opening the second page and sending a single message:

just get event "foo" with message: xxx
just get event "foo" with message: xxx
Window/tabs tracking
count: 5
open: {id: 149a1290-025d-40ec-a1e9-4381ed3bb752} [secondary]
track: secondary
message: Welcome from 768d6ff4-d494-4e0f-b527-f87ff46359de
send: Thanks
track: secondary
message: Welcome from 768d6ff4-d494-4e0f-b527-f87ff46359de
track: secondary
message: Welcome from 768d6ff4-d494-4e0f-b527-f87ff46359de
track: secondary
message: Welcome from 768d6ff4-d494-4e0f-b527-f87ff46359de

Maybe you don't see this in your project because you don't log message like my demo do.

@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

I think that if there are two iframes it breaks. It should work the same with the old setup with having two iframes. So you need to check the domain of the iframe.

I used my demo that had two iframes for terminal.jcubic.pl ad jcubic.pl and I used localhost to connect.

@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

I'm also wondering if there is no security issue with your solution. This is done only by a single browser but some applications may send sensitive information and they can leak out. If someone will open a connection without an iframe.

@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

If you really need this, this needs to be protected and restricted to specified Domains only. Domains that are in sysend.proxy. Only those domains should be allowed to listen to events.
And the bug with duplicated events needs to be fixed.


var key = data && data.name;
if (callbacks[key])
invoke(key, data.data);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And please use curly braces on if statements. Maybe I should set up ESLint for this.

@gerneio
Copy link
Author

gerneio commented Feb 13, 2022 via email

@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

If so I will merge this PR into an experimental branch. And investigate. Right now it doesn't work and break existing code. So it can't land into the master branch unless bugs are fixed.

@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

If you create a PR, you expect it to have no visible bug at least.

@jcubic
Copy link
Owner

jcubic commented Feb 13, 2022

Can you change the target branch to single-iframe-experimet? I'm not able to change it. And if I merge from the command line your commits and contribution are lost.

@jcubic
Copy link
Owner

jcubic commented Mar 13, 2022

I think that security is not a problem, because even right now attackers can listen to events.

jcubic added a commit that referenced this pull request Mar 13, 2022
Thanks for @gerneio in #30 for the idea for the change
It solves #29 and #28
@jcubic jcubic closed this Mar 13, 2022
jcubic added a commit that referenced this pull request Mar 13, 2022
Thanks for @gerneio in #30 for the idea for the change
It solves #29 and #28
@jcubic
Copy link
Owner

jcubic commented Mar 13, 2022

I needed to move development to devel branch because there was some issue with this solution. I thought that it was working but it's not. I have one of two behaviors no message sent or duplicated message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants