[Webkit-unassigned] [Bug 192736] HTTPS Upgrade: Use full sqlite upgrade list

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=192736

--- Comment #7 from Andy Estes <aestes at apple.com> ---
Comment on attachment 357507
  --> https://bugs.webkit.org/attachment.cgi?id=357507
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=357507&action=review

> Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.cpp:69
> +        if (!this->m_database->open(path)) {
> +            RELEASE_LOG_ERROR_IF_ALLOWED("NetworkHTTPSUpgradeChecker() - Unable to open database.");
> +            return;
> +        }
> +
> +        if (this->m_statement->prepare() != SQLITE_OK) {
> +            RELEASE_LOG_ERROR_IF_ALLOWED("NetworkHTTPSUpgradeChecker() - Unable to prepare statement.");
> +            return;
> +        }

Under what scenario would the HTTPS_UPGRADE feature be enabled with a read-only database in the WebKit bundle that can't be opened?

Same question for failures to prepare our hard-coded SQL query.

Can these be assertions instead?

> Source/WebKit/NetworkProcess/NetworkHTTPSUpgradeChecker.cpp:101
> +        if (!this->m_isSetupComplete) {
> +            RELEASE_LOG_IF_ALLOWED("query - Setup is not complete, ignoring query.");
> +            runCallbackOnMainThread(false);
> +            return;
> +        }

The logging here makes it sound like you are ignoring the query because setup is not yet complete, but that's impossible. By the time this lambda executes on the work queue setup will have completed (possibly with an error). I think what you really want to check is whether an error occurred in the setup lambda, which means m_isSetupComplete probably needs a better name.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20181218/c5910d52/attachment.html>