Add parameters parsing for cssfixme

[karlcow]

/cssfixme.php?url=http%3A%2F%2Fhallvord.com%2Ftemp%2Fmoz%2FcompatTesterTesting%2Fstyle.css

should be able to inject the CSS stylesheets inside the textarea.

Be careful with security issues.

Depends on #12

[magsout]

it is possible to explain this issue? ❤️

[miketaylr]

@magsout we're moving http://hallvord.com/temp/moz/cssfixme.php to webcompat.com/tools/cssfixme (it's a tool to add standards/unprefixed CSS to -webkit-prefix only code).

One of the features is that you can give it a URL via a get param, and it will automatically fetch that and put it in the tool for you -- that's what this bug is for.

[hallvors]

Basically, we want this URL to work:
https://webcompat.com/tools/cssfixme?url=http%3A%2F%2Fhallvord.com%2Ftemp%2Fmoz%2FcompatTesterTesting%2Fstyle.css

[karlcow]

Let's not deploy this for now. There are some security/usability issues with the code. I found new ones.

[karlcow]

• http://localhost:5000/tools/cssfixme?url=http://localhost gives the home page of the server you are on if any or a 504 Gateway issue for nginx
• http://localhost:5000/tools/cssfixme?url=http://127.0.0.1 gives the home page of the server you are on if any or a 504 Gateway issue for nginx
• http://localhost:5000/tools/cssfixme?url=http://www.mozilla.org injects the full HTML, we should check the content-type of the return document is text/css before injecting it
• http://localhost:5000/tools/cssfixme?url=www sends for me the home page of my router, because it seems the domain is defined locally. fun stuff.
• suggest to send in the template instead of an empty area an error message saying that the URL is invalid.

[miketaylr]

Let's not deploy this for now. There are some security/usability issues with the code. I found new ones.

It's too late, we already deployed. I'm going to disable it on master now. @karlcow, please file new issues.

[miketaylr]

Actually, no need for new issues @karlcow. This is the right place.

[miketaylr]

OK, fix deployed. I'm not even sure this feature is worth the trouble, given its ability to take down our server if we don't do it correctly:

2015/12/18 01:13:54 [error] 4971#0: *REDACTED upstream timed out (110: Connection timed out) while reading response header from upstream, client: REDACTED, server: webcompat.com, request: "GET /tools/cssfixme?url=http://127.0.0.1 HTTP/1.1", upstream: REDACTED, host: "webcompat.com"

[karlcow]

Thanks! @miketaylr

[miketaylr]

I'm inclined to close this as WONTFIX, given that it's some nice sugar on top of the core functionality that is working. We have a lot of other features to worry about, that (in theory) don't have the same potential for dangerous security implications.

I'm not even sure we should host https://github.com/webcompat/css-fixme/blob/master/cssfixme.php in our org's repo. It's just not safe code as-is. :(

[karlcow]

Agreed with @miketaylr

I propose in the meantime, we close that feature.
If one day, we have a rock solid implementation/idea for it, we can still reopen. Too many unknowns for now. This includes.

• verify the URI exist
• verify it sends back a 200
• verify text/css content-type
• verify it's not a private ip and/or local server address
• verify it's a CSS
• and probably a couple of others.

[miketaylr]

Filed webcompat/css-fixme#16 which is related.

[miketaylr]

OK, let's close for now.

[hallvors]

:(
It is a useful thing to be able to send links to webmasters that show their code already fixed. Oh well.. Too many issues. I'll consider other ways to do something..

[miketaylr]

@hallvors we can re-open (and re-add the feature) in the future, as long as we get security right.