Skip to content
New issue

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

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

Already on GitHub? # to your account

Setting the origin of the request #69

Closed
zastrixarundell opened this issue Feb 28, 2020 · 7 comments
Closed

Setting the origin of the request #69

zastrixarundell opened this issue Feb 28, 2020 · 7 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed resolved the issue was resolved

Comments

@zastrixarundell
Copy link
Contributor

zastrixarundell commented Feb 28, 2020

Not really an issue but would be an awesome upgrade. I've tested the websocket client on my server in production and I get the following (you can skip after the block of code, I'll provide a TL;DR):

2020-02-28T21:37:23.790390+00:00 app[web.1]: 21:37:23.790 [error] Could not check origin for Phoenix.Socket transport.

2020-02-28T21:37:23.790398+00:00 app[web.1]: 

2020-02-28T21:37:23.790398+00:00 app[web.1]: Origin of the request: https://github.com/gilmaimon/TinyWebsockets

2020-02-28T21:37:23.790399+00:00 app[web.1]: 

2020-02-28T21:37:23.790399+00:00 app[web.1]: This happens when you are attempting a socket connection to

2020-02-28T21:37:23.790400+00:00 app[web.1]: a different host than the one configured in your config/

2020-02-28T21:37:23.790400+00:00 app[web.1]: files. For example, in development the host is configured

2020-02-28T21:37:23.790400+00:00 app[web.1]: to "localhost" but you may be trying to access it from

2020-02-28T21:37:23.790401+00:00 app[web.1]: "127.0.0.1". To fix this issue, you may either:

2020-02-28T21:37:23.790401+00:00 app[web.1]: 

2020-02-28T21:37:23.790401+00:00 app[web.1]:   1. update [url: [host: ...]] to your actual host in the

2020-02-28T21:37:23.790402+00:00 app[web.1]:      config file for your current environment (recommended)

2020-02-28T21:37:23.790402+00:00 app[web.1]: 

2020-02-28T21:37:23.790403+00:00 app[web.1]:   2. pass the :check_origin option when configuring your

2020-02-28T21:37:23.790403+00:00 app[web.1]:      endpoint or when configuring the transport in your

2020-02-28T21:37:23.790404+00:00 app[web.1]:      UserSocket module, explicitly outlining which origins

2020-02-28T21:37:23.790404+00:00 app[web.1]:      are allowed:

2020-02-28T21:37:23.790405+00:00 app[web.1]: 

2020-02-28T21:37:23.790405+00:00 app[web.1]:         check_origin: ["https://example.com",

2020-02-28T21:37:23.790405+00:00 app[web.1]:                        "//another.com:888", "//other.com"]

Basically the issue is that the origin is https://github.com/gilmaimon/TinyWebsockets and I need to add it to the accepted origins in my app. It would be awesome to make it configurable.

@gilmaimon
Copy link
Owner

I added the Origin header recently to solve a problem for a user, but you are right that it probably should be configurable. There is a feature to add custom headers to the request, but that will result in duplicate Origin header rather than replace the default one. I'll make that configurable, but it might take couple of days.

Thanks, Gil.

@gilmaimon gilmaimon added the enhancement New feature or request label Feb 29, 2020
@gilmaimon
Copy link
Owner

Hi,
The library right now is in maintance mode, meaning It's hard for me to find time to add and test new features. So i'm opening this request for public contribuation (it always was public, but now I will mark it appropriately). If anyone has the time to do it it will be very helpful and appriciated.

What needs to be done:
So, one option is to have a setOrigin method. I don't really like this soulution as someone in the future will want to change other headers and it can be a mess to add new method and field for each header.
There is a addHeader method. Those customHeaders are passed to generateHandshake which will construct an HTTP request string and will also consider those custom header values (here)
I think that instead of adding the (non custom) headers every time, we should first check if they are already in the customHeaders . So for example, this line:

        handshake += "Origin: https://github.com/gilmaimon/TinyWebsockets\r\n";

will only get executed if "handshake" not in customHeaders.

This is my opinion and it is open for discussion and contribuation.

Thanks,
Gil.

@zastrixarundell
Copy link
Contributor Author

zastrixarundell commented Apr 25, 2020

Hi, thanks for the reply. I have created a PR #74 but it is untested as for now. (I need to create a websocket server first).

I will update the PR and this issue post as soon as I see if it's working.

Edit:

I had gone with the 2nd method you had written. Rather than creating a method for every header type, this firstly writes the user-created headers and then it adds default headers if they're not declared.

@zastrixarundell
Copy link
Contributor Author

zastrixarundell commented Apr 25, 2020

Update:

This was tested on an ESP8266 (NodeMCU 1 copy) and I was able to successfully overwrite the Origin header simply with:

  client.addHeader("Origin", "https://localhost");

  client.connect(websocketUrl);

When I didn't write the addHeader method, it would use the default https://github.com/gilmaimon/TinyWebsockets value.

@gilmaimon
Copy link
Owner

Awesome, the PR was merged and I will publish a version in a few moments.

Thank you!

@gilmaimon gilmaimon added the resolved the issue was resolved label Apr 25, 2020
@zastrixarundell
Copy link
Contributor Author

Oh and @gilmaimon could you add me to the contributor list in README as well? Thanks in advance!

@gilmaimon
Copy link
Owner

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed resolved the issue was resolved
Projects
None yet
Development

No branches or pull requests

2 participants