Add option to expose WebpackDevServer publicly for non-localhost development #1621

jankeromnes · 2019-01-11T14:01:16Z

If you'd like to use perf.html via URLs that are not localhost (e.g. from behind a proxy, or from another device) I suggest to expose the web application publicly like so:

PERFHTML_PUBLICHOST=true yarn start

codecov · 2019-01-11T14:25:06Z

Codecov Report

Merging #1621 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1621   +/-   ##
=======================================
  Coverage   81.04%   81.04%           
=======================================
  Files         178      178           
  Lines       11696    11696           
  Branches     2843     2846    +3     
=======================================
  Hits         9479     9479           
  Misses       2013     2013           
  Partials      204      204

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e709914...f163d31. Read the comment docs.

codecov · 2019-01-11T14:25:06Z

Codecov Report

Merging #1621 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1621   +/-   ##
=======================================
  Coverage   82.05%   82.05%           
=======================================
  Files         176      176           
  Lines       11908    11908           
  Branches     2901     2898    -3     
=======================================
  Hits         9771     9771           
  Misses       1949     1949           
  Partials      188      188

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db03840...04706a6. Read the comment docs.

julienw · 2019-01-11T14:27:48Z

Some context for my teammates:
@jankeromnes now works for typefox, the company behinds gitpod.io. We chatted a bit today about how to support this for our project, using an URL like https://gitpod.io#https://github.com/devtools-html/perf.html/ for easy contribution.

One issue is how we access the dev server: obviously the server won't be accessible from the docker container as it binds on localhost. gitpod comes with a system to forward ports, but the webpack dev server has a system to checks the Host to prevent security problems. See this posts about this issue:

The PR here disables this check and I'm reluctant to do it. Ideally gitpod should tell you which host will be used, and we could use it when configuring the server.

jankeromnes · 2019-01-11T14:29:35Z

The PR here disables this check and I'm reluctant to do it.

Note: This PR only disables the check if you explicitly set PERFHTML_PUBLICHOST=true (one typo, and you're back to the safer situation & the server won't get exposed beyond localhost).

julienw

I think the proposed changes make it safer and still not overengineered :)
Then a user could pass the expected host through PERFHTML_PUBLICHOST (maybe with a bit of guessing in the case of gitpod - but should be doable) and have it exposed properly.

server.js

julienw · 2019-01-11T14:32:30Z

server.js

@@ -13,6 +15,7 @@ const localConfigExists = fs.existsSync(
 const serverConfig = {
  contentBase: config.output.path,
  publicPath: config.output.publicPath,
+  disableHostCheck: process.env.PERFHTML_PUBLICHOST === 'true',


remove this and add public: host

Removed. But is public: host still needed here? Or maybe host should be added to allowedHosts?

server.js

julienw · 2019-01-11T14:33:48Z

server.js

  }
-);
+  console.log(barAscii);
+});


indeed it's weird your prettier run changed everything here. Meh :)

julienw · 2019-01-11T15:00:04Z

@gregtatum I'd like your second opinion on this :) here is how you can try the issue we're facing:

open https://gitpod.io/#https://github.com/devtools-html/perf.html/
run yarn then yarn start in a terminal (inside the interface of course)
run gp forward-port 4242 4243 in another terminal
=> you should have a link to the ports on the bottom bar
expose the port 4243 from the popin appearing at the top of the window
open a new window from the popin appearing at the top of the window
=> We get "Invalid Host header"

gregtatum · 2019-01-15T14:51:45Z

I don't think I understand enough about this issue to really weigh in. I did the STR, but I'm not sure what's really going on behind the scenes with the tech. What is even sending out "Invalid Host Header"?

jankeromnes · 2019-01-15T15:01:11Z

@gregtatum Thanks for looking into this!

What is even sending out "Invalid Host Header"?

That's WebpackDevServer, because it doesn't accept connections that use domains other than localhost. That's problematic when you develop with live previews / proxies / multiple devices (as is the case for example in Gitpod, which shows running web apps via a preview URL).

This is essentially why I added support for making perf.html accept connection that use non-localhost domains.

jankeromnes · 2019-01-15T15:10:18Z

To me, these are the relevant options to address this:

Don't address it: We don't want anyone to develop perf.html using anything else than localhost
Option to allow only specific non-localhost access: E.g. PERFHTML_PUBLICHOST="4242-abcd-ef01-2345-complicated-preview-domain" yarn start
Option to allow any non-localhost access: E.g. PERFHTML_PUBLICHOST=true yarn start
Completely disable host-checking for development: E.g. yarn start works everywhere

This PR implements Option 3. @julienw seems to prefer Option 2.

mstange · 2019-01-15T15:35:20Z

I don't really have an opinion on this either. I'd even be fine with a gitpod-specific command that looks at the GITPOD_WORKSPACE_URL env var, e.g. yarn gitpod-start.

julienw · 2019-01-15T16:35:07Z

That special command could do:

gp forward-port 4242 4243
PERFHTML_PUBLICHOST="4243-$GITPOD_WORKSPACE_ID.ws-eu.gitpod.io" yarn start

Assuming this ws-eu is something constant.

jankeromnes · 2019-01-16T09:04:23Z

Interesting idea, thanks! We'll still need to implement PERFHTML_HOST env variable support for Option 2 (I'll amend this PR), but this would indeed solve non-localhost development for any live preview / proxy / cross-device scenario where you know the host in advance, and won't require using disableHostCheck: true.

Assuming this ws-eu is something constant.

Unfortunately, it is not constant (e.g. if you're in Asia, it will be ws-ap). However, there is probably a way to make Gitpod expose these preview URLs. I'll investigate.

julienw · 2019-01-16T14:23:18Z

I got 1 thought and 1 remark.

First, the thought: after looking at this again, I now think this is probably good enough to do what @janx proposed initially. That we're not having this setup by default is probably good enough for security. I'd still be happier with the other solution though. Especially that the next remark could make it a lot easier:

Indeed I noticed that the allowedHosts property accepts wildcards, so we could use something generic like .gitpod.io instead of having to guess the full name.

jankeromnes · 2019-01-16T15:13:04Z

Indeed I noticed that the allowedHosts property accepts wildcards, so we could use something generic like .gitpod.io instead of having to guess the full name.

Wow, that's a really cool find! This looks like a much cleaner solution indeed. 😄

jankeromnes · 2019-01-16T15:45:20Z

@julienw Thanks again for your advice! I've implemented non-localhost support without using disableHostCheck at all, please take another look when you have time.

(Also, I haven't added .gitpod.io to allowedHosts yet because I'd like to implement Gitpod support in a separate pull request.)

jankeromnes · 2019-01-21T18:20:14Z

(Rebased on top of latest master.)

julienw

I did some small suggestions, I hope you'll find them OK :)

CONTRIBUTING.md

server.js

jankeromnes · 2019-01-23T16:08:18Z

@julienw Many thanks! 😄 The suggestions make sense to me, so I've applied them. I'll also rebase the commits now.

…lopment

Co-Authored-By: jankeromnes <jan.keromnes@typefox.io>

jankeromnes · 2019-01-23T16:51:24Z

🎉 🎉 🎉 Thanks a lot @julienw! 😄

julienw requested changes Jan 11, 2019

View reviewed changes

jankeromnes force-pushed the publichost branch from f163d31 to e0edc6c Compare January 16, 2019 15:40

julienw self-requested a review January 18, 2019 15:57

jankeromnes force-pushed the publichost branch 2 times, most recently from ff5d73a to 49f2585 Compare January 21, 2019 18:15

jankeromnes force-pushed the publichost branch from 49f2585 to 2cfe7eb Compare January 23, 2019 08:46

julienw requested changes Jan 23, 2019

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

server.js Outdated Show resolved Hide resolved

jankeromnes and others added 3 commits January 23, 2019 16:09

Add option to expose WebpackDevServer publicly for non-localhost deve…

27a8702

…lopment

Run 'yarn lint --fix'

bef9537

Apply suggestions from code review

04706a6

Co-Authored-By: jankeromnes <jan.keromnes@typefox.io>

jankeromnes force-pushed the publichost branch from 94f11f8 to 04706a6 Compare January 23, 2019 16:09

julienw approved these changes Jan 23, 2019

View reviewed changes

julienw merged commit 1e3379b into firefox-devtools:master Jan 23, 2019

jankeromnes deleted the publichost branch January 23, 2019 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to expose WebpackDevServer publicly for non-localhost development #1621

Add option to expose WebpackDevServer publicly for non-localhost development #1621

jankeromnes commented Jan 11, 2019 •

edited

Loading

codecov bot commented Jan 11, 2019

codecov bot commented Jan 11, 2019 •

edited

Loading

julienw commented Jan 11, 2019

jankeromnes commented Jan 11, 2019 •

edited

Loading

julienw left a comment

julienw Jan 11, 2019

jankeromnes Jan 16, 2019

julienw Jan 11, 2019

julienw commented Jan 11, 2019 •

edited

Loading

gregtatum commented Jan 15, 2019

jankeromnes commented Jan 15, 2019 •

edited

Loading

jankeromnes commented Jan 15, 2019

mstange commented Jan 15, 2019

julienw commented Jan 15, 2019

jankeromnes commented Jan 16, 2019 •

edited

Loading

julienw commented Jan 16, 2019

jankeromnes commented Jan 16, 2019

jankeromnes commented Jan 16, 2019

jankeromnes commented Jan 21, 2019

julienw left a comment

jankeromnes commented Jan 23, 2019

jankeromnes commented Jan 23, 2019

Add option to expose WebpackDevServer publicly for non-localhost development #1621

Add option to expose WebpackDevServer publicly for non-localhost development #1621

Conversation

jankeromnes commented Jan 11, 2019 • edited Loading

codecov bot commented Jan 11, 2019

Codecov Report

codecov bot commented Jan 11, 2019 • edited Loading

Codecov Report

julienw commented Jan 11, 2019

jankeromnes commented Jan 11, 2019 • edited Loading

julienw left a comment

Choose a reason for hiding this comment

julienw Jan 11, 2019

Choose a reason for hiding this comment

jankeromnes Jan 16, 2019

Choose a reason for hiding this comment

julienw Jan 11, 2019

Choose a reason for hiding this comment

julienw commented Jan 11, 2019 • edited Loading

gregtatum commented Jan 15, 2019

jankeromnes commented Jan 15, 2019 • edited Loading

jankeromnes commented Jan 15, 2019

mstange commented Jan 15, 2019

julienw commented Jan 15, 2019

jankeromnes commented Jan 16, 2019 • edited Loading

julienw commented Jan 16, 2019

jankeromnes commented Jan 16, 2019

jankeromnes commented Jan 16, 2019

jankeromnes commented Jan 21, 2019

julienw left a comment

Choose a reason for hiding this comment

jankeromnes commented Jan 23, 2019

jankeromnes commented Jan 23, 2019

jankeromnes commented Jan 11, 2019 •

edited

Loading

codecov bot commented Jan 11, 2019 •

edited

Loading

jankeromnes commented Jan 11, 2019 •

edited

Loading

julienw commented Jan 11, 2019 •

edited

Loading

jankeromnes commented Jan 15, 2019 •

edited

Loading

jankeromnes commented Jan 16, 2019 •

edited

Loading