Added web root feature

[MarcinGmurczyk]

Hi

I want to propose some changes in script so that reverse proxy functionality is possible for user. Link example is for nginx but feature is also avalaible in Apache.

Reverse proxy gives user ability to access web application through defined address. If user want access through WAN there is no need to open any ports on working machine so reverse proxy increases security.

Simple nginx conf for reverse proxy can looks like this

To enable reverse proxy user has to add variable "web_root" in conf.json, like that
{
"web_root" : "/script-server"
}

With reverse proxy set as in this example user can access web application in few ways (of course launcher.py must be running, through systemd, supervisor etc.):
localhost/script-server on local machine
192.168.0.XX/script-server on LAN machine
domain.com/script-server on WAN machine

I'm using reverse proxy on my machine with multiple applications, mainly because I do not need to open so many ports on router and also it is easier to remember some string instead of port number.

I want to stress out that I do not know Tornado, I did my best to make things works as intended. As fair as I know these changes does not broke any functionalities. All test are passing. It might be wise to get code review from someone who actually knows how reverse proxy should be implemented with Tornado.

[bugy]

Hi Marcin, thank you very much for the PR! I'll review the changes and test it locally today.

[MarcinGmurczyk]

Unfortunately these changes are breaking user script execution. Script itself is executed but application cannot get access to given process id.

Error:
index.js:586 WebSocket connection to 'ws://localhost/script-server/scripts/execute/io/6260' failed: Error during WebSocket handshake: Unexpected response code: 400

There is no such problem when reverse proxy is disabled.

Probably the issue here is with not properly set url for retrieving script output, but I cannot locate it.

[bugy]

Unfortunately these changes are breaking user script execution. Script itself is executed but application cannot get access to given process id.
I'll have a look

[bugy]

Hi @MarcinGmurczyk, I've checked your changes and from general perspective they look good. However, I think, that specifying web_root config is redundant in a script-server (using relative paths everything should work out of the box).

I've tried to improve nginx conf, so it's responsible for URL substitution itself. Please see updated conf:
https://gist.github.com/bugy/df2672824a43cb8f8ed599972e7a739c
3 main changes:

1. '/' sign at the end of proxy_pass. This is a feature of nginx. In case of such URL, nginx performs URL replacements
2. 'Host' header is now proxy host (probably it's not needed at all). Otherwise, when Script-server builds redirect URLs (for login for example), it cannot build proper full URL
3. I've added a config block for web sockets, so now web socket connection (ws://) is properly established through http proxy.

With these config changes and Script-server impovements in 1da9d86 commit, everything worked fine for me (login, redirect, execution, reading user input, etc.)

I'm not nginx expert, so may be my configuration has some other issues. What do you think about this configuration? If it is fine and suits you, I'll put them in a wiki as an example for reverse proxy.

[MarcinGmurczyk]

Hi @bugy
I've merged your commit into my branch. Your solution is much simpler and cleaner - thanks!
I also removed some garbage code with web_root declarations.

With your nginx conf reverse proxy is working great.

One last thing that for sure need to be done here is set proper proxied url's for dynamic image sources. Please run script-server and hover mouse over cookie, the error here:
Failed to load resource: the server responded with a status of 404 (Not Found) /images/cookie.png

Same error with search/clear images. Do you think you could fix these errors?

[bugy]

Hi, thanks for reporting the problem with images. I've fixed it.
Is this pull request still valid?

PS in your codebase JS paths in index.html file are different from mine. Is it intended?

[MarcinGmurczyk]

Good catch, I've fixed JS paths in index.html.

I believe reverse proxy is now available.
And since you resolved this feature with nginx configuration and url creation handle with commit #82 maybe you should just merge #82, add nginx conf to wiki and close this PR without merging (this way you could preserve clean commits history).

It's entirely up to you :)

[bugy]

Thanks for checking my fixes. Short information with nginx config is in a wiki.

Since there are no remaining changes in PR, I'll just close it.
And I would be really glad to receive any new feedback or PRs :)