Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Issues with proxied @import https urls #741

Closed
wzrdtales opened this issue Mar 9, 2016 · 4 comments · Fixed by #748
Closed

Issues with proxied @import https urls #741

wzrdtales opened this issue Mar 9, 2016 · 4 comments · Fixed by #748
Labels

Comments

@wzrdtales
Copy link
Contributor

Problem description

If you're using a http proxy the current implementation causes trouble and renders it completely unusable.

Example error:

File: assets/vendor.css
Broken @import declaration of "https://fonts.googleapis.com/css?family=Lato:400,700,400italic,700italic&subset=latin" - write EPROTO

Gist: https://gist.github.com/wzrdtales/621ff62d38bb94737215

Details

The failure is here;

https://github.com/jakubpawlowicz/clean-css/blob/master/lib/imports/inliner.js#L271-L273

This can't work and wont work if there is only a http proxy, but not a https one. And this is common for mostly all the time, to have just a http proxy.

Possible solutions

Two options to fix this:

  • Fix the check for http and https in combination with the proxy.
  • Use the request lib instead

Using the request lib would be also be more convenient for the users, as it automatically parses http_proxy and https_proxy environment variables and handles many edge cases already. Thus the users never need to set any proxy configurations for clean-css if they are already defined them in their environment.

@wzrdtales wzrdtales changed the title Issues with proxied https urls Issues with proxied @import https urls Mar 9, 2016
@jakubpawlowicz
Copy link
Collaborator

Could you please provide a test case with CSS and system proxy settings? I'll try to replicate it locally first.

@wzrdtales
Copy link
Contributor Author

@jakubpawlowicz Really thought the error message is already enough to reproduce it.

Here is a gist for you https://gist.github.com/wzrdtales/621ff62d38bb94737215

@jakubpawlowicz
Copy link
Collaborator

Thank you! It's always easier to reproduce an issue with a code snippet to make sure we are on the same page. Can see it failing now.

wzrdtales added a commit to wzrdtales/clean-css that referenced this issue Mar 16, 2016
When an @import used a https url, the false protocol was tried to be used for the http proxy.

Additionally request has replaced a direct http.get and https.get call, to automatically handle proxy environment variables
and other possible edge cases.

fixes clean-css#741
wzrdtales added a commit to wzrdtales/clean-css that referenced this issue Mar 16, 2016
When an @import used a https url, the false protocol was tried to be used for the http proxy.

fixes clean-css#741
wzrdtales added a commit to wzrdtales/clean-css that referenced this issue Mar 27, 2016
When an @import used a https url, the false protocol was tried to be used for the http proxy.

fixes clean-css#741
jakubpawlowicz pushed a commit that referenced this issue Apr 1, 2016
When an @import used a https url, the false protocol was tried to be used for the http proxy.

fixes #741
jakubpawlowicz added a commit that referenced this issue Apr 1, 2016
jakubpawlowicz added a commit that referenced this issue Apr 1, 2016
@jakubpawlowicz
Copy link
Collaborator

Released in 3.4.11, thanks @wzrdtales!

silverwind pushed a commit to silverwind/clean-css that referenced this issue Sep 25, 2016
When an @import used a https url, the false protocol was tried to be used for the http proxy.

fixes clean-css#741
silverwind pushed a commit to silverwind/clean-css that referenced this issue Sep 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants