New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 724905 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

PasswordProtectionService stores binary data in prefs

Project Member Reported by sa...@chromium.org, May 22 2017

Issue description

Prefs only supports data types serializable as JSON. Storing a base::Value of type BINARY renders prefs unserializable.

PasswordProtectionService stores a serialized proto as a binary base::Value in content settings (which is backed by prefs). I'm fairly certain this triggered issue 721685.

Please use an alternative format (e.g. base64 encode the serialized proto).
 
Status: Assigned (was: Available)
sammc@, thanks for pointing this out! 
Do you mean applying base::Base64Encode(..) to the serialized string and then store it as STRING?
Labels: SafeBrowsing-Triaged

Comment 4 by sa...@chromium.org, May 22 2017

Yes.
Project Member

Comment 5 by bugdroid1@chromium.org, May 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee69e6f69b1498d2f7b91a8e8ffc2a77603951e8

commit ee69e6f69b1498d2f7b91a8e8ffc2a77603951e8
Author: jialiul <jialiul@chromium.org>
Date: Mon May 22 23:58:56 2017

Change string data instead of binary in content settings.

BUG= 724905 

Review-Url: https://codereview.chromium.org/2895323002
Cr-Commit-Position: refs/heads/master@{#473750}

[modify] https://crrev.com/ee69e6f69b1498d2f7b91a8e8ffc2a77603951e8/components/safe_browsing/password_protection/password_protection_service.cc

Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, May 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/537415ea77b5938871ca199b1f68253bcd657da6

commit 537415ea77b5938871ca199b1f68253bcd657da6
Author: Sam McNally <sammc@chromium.org>
Date: Tue May 23 10:30:17 2017

Check that prefs are serializable in TestingPrefStore and JsonPrefStore.

Bug:  724905 
Change-Id: Ic813e70973ba9aef1c88278e51d4424547d245b8
Reviewed-on: https://chromium-review.googlesource.com/509409
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#473849}
[modify] https://crrev.com/537415ea77b5938871ca199b1f68253bcd657da6/components/prefs/json_pref_store.cc
[modify] https://crrev.com/537415ea77b5938871ca199b1f68253bcd657da6/components/prefs/testing_pref_store.cc
[modify] https://crrev.com/537415ea77b5938871ca199b1f68253bcd657da6/components/prefs/testing_pref_store.h

Sign in to add a comment