webhintio / hint

💡 A hinting engine for the web
https://webhint.io/
Apache License 2.0
3.63k stars 685 forks source link

[Bug] Redirect from http to https shold not be reported as error #3131

Closed kolappannathan closed 2 years ago

kolappannathan commented 5 years ago

🐛 Bug report

Description

Currently if a webpage redirects from http to https it is displayed as an error in Performance -> no-http-redirects. This should not be the case.

Example: https://webhint.io/scanner/ce2ee61b-9370-4c43-ad1f-29d7cec6340a#hint-no-http-redirects-1

Environment

webhint configuration

webhint’s configuration ```json [{"hintsTimeout":120000,"parsers":["manifest"],"hints":{"apple-touch-icons":"warning","button-type":"error","compat-api/css":"error","compat-api/html":"error","content-type":"error","css-prefix-order":"error","create-element-svg":"error","disown-opener":"error","highest-available-document-mode":"error","http-cache":"error","http-compression":["error",{"html":{"zopfli":false,"brotli":false}}],"https-only":"error","manifest-app-name":"warning","manifest-exists":"off","manifest-file-extension":"warning","manifest-is-valid":"error","meta-charset-utf-8":"error","meta-viewport":"error","minified-js":"error","no-bom":"error","no-broken-links":"off","no-disallowed-headers":["error",{"ignore":["Server"]}],"no-friendly-error-pages":"warning","no-html-only-headers":"error","no-http-redirects":["error",{"max-target-redirects":1}],"no-p3p":"warning","no-protocol-relative-urls":"error","performance-budget":["error",{"connectionType":"4G","loadTime":5}],"scoped-svg-styles":"error","sri":"error","strict-transport-security":"error","validate-set-cookie-header":"error","x-content-type-options":"error"},"formatters":["summary"],"connector":{"options":{"headless":true},"name":"puppeteer"},"browserslist":["defaults","not IE 11"],"ignoredUrls":[{"domain":"^https?://cdn\\.jsdelivr\\.net/.*","hints":["*"]},{"domain":"^https?://.*\\.google-analytics\\.com/.*","hints":["*"]},{"domain":"^https?://fonts\\.googleapis\\.com/.*","hints":["*"]}]},{"hintsTimeout":170000,"parsers":["manifest"],"hints":{"axe/aria":"error","axe/color":"error","axe/forms":"error","axe/keyboard":"error","axe/language":"error","axe/name-role-value":"error","axe/other":"error","axe/parsing":"error","axe/semantics":"error","axe/sensory-and-visual-cues":"error","axe/structure":"error","axe/tables":"error","axe/text-alternatives":"error","axe/time-and-media":"error","html-checker":"off","image-optimization-cloudinary":["error",{"threshold":5}],"no-vulnerable-javascript-libraries":"error","ssllabs":"error","stylesheet-limits":"error"},"formatters":["summary"],"connector":{"options":{"headless":true},"name":"puppeteer"},"browserslist":["defaults","not IE 11"],"ignoredUrls":[{"domain":"^https?://cdn\\.jsdelivr\\.net/.*","hints":["*"]},{"domain":"^https?://.*\\.google-analytics\\.com/.*","hints":["*"]},{"domain":"^https?://fonts\\.googleapis\\.com/.*","hints":["*"]}]}] ```
antross commented 5 years ago

Thanks @kolappannathan!

I agree this seems a bit odd to report for the top-level page. It should still make sense for sub-resources as the sub-resource should point to HTTPS to begin with.

@molant what do you think?

molant commented 5 years ago

This redirect still has a perf impact (need to establish a new connection to the server) that I don't think should be masked to the user. I've seen many shared URLs that use http instead of https. If the user knows that a website supports https then I'd suggest that the target URL should be the https one.

That said, one thing we could do for the particular github.com case (and many other big sites) is to use the HSTS preload list. The browsers will automatically use the https version if the domain is in there even if the user types http.

What do you think?

kolappannathan commented 5 years ago

@molant In this case, I type only github.com and webhint automatically took it as "http".

Malvoz commented 5 years ago

@molant

This redirect still has a perf impact

I think it's best to aim to prioritize privacy over performance. While HSTS preloading is great, it isn't guaranteed; not all websites are eligible, and even when they are, inclusion may take some time. Redirects serves as a privacy-preserving fallback.

Somewhat ironically, redirects are also a HSTS Preload requirement:

https://hstspreload.org/#submission-requirements

Submission Requirements

[...]

  1. Redirect from HTTP to HTTPS on the same host, if you are listening on port 80.

@kolappannathan your comment https://github.com/webhintio/hint/issues/3131#issuecomment-545860600 seems to be a separate issue though, which is regarding the webhint website and how the URL input is treated by default.

Malvoz commented 5 years ago

Also isn't this hint (hint-no-http-redirects) contradictory to hint-https-only?

molant commented 5 years ago

not all websites are eligible,

Yup, still need to tweak a few things. We've had problems with the certificates in the past and it was a nightmare to get everything working again 😨

Somewhat ironically, redirects are also a HSTS Preload requirement:

Isn't that to make sure only content over https is served already before being in the list? Once they are in the list and the browser has loaded it, it should navigate directly to https.

Also isn't this hint (hint-no-http-redirects) contradictory to hint-https-only?

Having a redirect on the target has a perf impact. Right now all redirects are warnings (happy to review/update the severities and have them more granular). I think people that know a website is https only should analyze the final url. That's the one that is going to load faster and that should be shared (most of the times). All the other alternatives (https, www, shortlinks, etc.) are just going to impact the load time and people should be aware of this.

Malvoz commented 5 years ago

redirects are also a HSTS Preload requirement:

Isn't that to make sure only content over https is served already before being in the list? Once they are in the list and the browser has loaded it, it should navigate directly to https.

The Continued requirements section says:

"You must make sure your site continues to satisfy the submission requirements at all times." 👀