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

Fix HTTPS cache problems on pages with certificate errors #125

Closed
GoogleCodeExporter opened this issue Aug 28, 2015 · 3 comments
Closed
Milestone

Comments

@GoogleCodeExporter
Copy link

This is an optional fix. By default Chromium disables caching when there is certificate error. This patch will fix the HTTPS caching only when ApplicationSettings "ignore_certificate_errors" is set to True.

See http://www.magpcss.org/ceforum/viewtopic.php?f=6&t=11609

The patch suggested in the linked topic works fine after making some minor
changes.

Index: net/http/http_cache_transaction.cc
===================================================================
--- http_cache_transaction.cc   (revision 241641)
+++ http_cache_transaction.cc   (working copy)
@@ -2240,7 +2240,8 @@
   // reverse-map the cert status to a net error and replay the net error.
   if ((cache_->mode() != RECORD &&
        response_.headers->HasHeaderValue("cache-control", "no-store")) ||
-      net::IsCertStatusError(response_.ssl_info.cert_status)) {
+       (!cache_->GetSession()->params().ignore_certificate_errors &&
+       net::IsCertStatusError(response_.ssl_info.cert_status))) {
     DoneWritingToEntry(false);
     ReportCacheActionFinish();
     if (net_log_.IsLoggingAllEvents())

The patch was verified to work with CEF 3 branch 1650 revision 1646. The https
web server was nginx running on Ubuntu.

Patch instructions available here (both BuildOnLinux and BuildOnWindows wiki pages).

Original issue reported on code.google.com by czarek.t...@gmail.com on 27 May 2014 at 8:38

@cztomczak
Copy link
Owner

This patch is currently disabled in v55 on Windows/Mac. Only Linux has this applied.

@cztomczak
Copy link
Owner

cztomczak commented May 22, 2018

This patch is now be disabled by default since v66 release. See commit f87422b. We plan on using Spotify builds on Linux and all platforms and this patch was not accepted into upstream. API docs and migration guide doc were updated in commit 1846d64 .

@cztomczak cztomczak modified the milestones: v55, v66 May 22, 2018
@cztomczak
Copy link
Owner

Maintaining such patch is costly, so it will be disabled unless there are parties interested in having this enabled in official releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants