New issue
Advanced search Search tips

Issue 659645 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug
V1

Blocking:
issue 660007



Sign in to add a comment

authpolicy: Fix preg_parser and registry_dict maintainance issues

Project Member Reported by ljusten@chromium.org, Oct 26 2016

Issue description

Right now preg_parser and registry_dict are stripped-down and slightly modified copies of Chromium versions. Changes to either side won't propagate. To fix this, consider putting the two classes into libchrome and use patches to patch up the differences. This way, Chromium OS gets changes when libchrome gets uprev'ed.
 
Labels: M-57
Copied  issue chromium:659645  to issue chromium:660006
Blocking: 660007
Labels: Enterprise-Triaged
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 17 2016

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

commit 32d00fec960c4005730cdb13a51a8416340638e9
Author: ljusten <ljusten@chromium.org>
Date: Thu Nov 17 17:54:58 2016

Push preg_parser and registry_dict changes upstream

preg_parser and registry_dict are used in Chromium OS by authpolicy
code to fetch policy from AD servers. Right now, a custom version of
the two files is checked into platform2 in Chromium OS. This CL
pushes changes made to these files upstream to Chromium, in particular
making them compile outside of Windows. The goal is to eventually use
them in libchrome, so that only one version of the code has to be
maintained and unit tests can be reused.

Both files are compiled for Windows and Chrome OS, even though only
the Windows version is used in Chromium. This is done to ensure the
file compiles OK and won't cause an issue downstream when used in
Chrome OS.

The i18n UTF16 ToLower method has been replaced by an ASCII method for two reasons:
- We don't need it, we always check for ASCII keys. Keys and values are actually converted to UTF8 later and compared using case-insensitive ASCII checks, so if it was a problem, it would already surface.
- More importantly, using i18n would introduce a dependency in Chrome OS on the third-party library icu and we don't want to pull this in.

RegistryDict::ConvertToJSON has been #ifdef'ed out in Chrome OS because it would pull in some dependencies and it is not needed right now.

BUG= chromium:659645 
TEST=Compiled on Linux and ran unit tests

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

[modify] https://crrev.com/32d00fec960c4005730cdb13a51a8416340638e9/components/policy/core/common/BUILD.gn
[modify] https://crrev.com/32d00fec960c4005730cdb13a51a8416340638e9/components/policy/core/common/policy_loader_win.cc
[modify] https://crrev.com/32d00fec960c4005730cdb13a51a8416340638e9/components/policy/core/common/policy_loader_win_unittest.cc
[rename] https://crrev.com/32d00fec960c4005730cdb13a51a8416340638e9/components/policy/core/common/preg_parser.cc
[rename] https://crrev.com/32d00fec960c4005730cdb13a51a8416340638e9/components/policy/core/common/preg_parser.h
[rename] https://crrev.com/32d00fec960c4005730cdb13a51a8416340638e9/components/policy/core/common/preg_parser_unittest.cc
[rename] https://crrev.com/32d00fec960c4005730cdb13a51a8416340638e9/components/policy/core/common/registry_dict.cc
[rename] https://crrev.com/32d00fec960c4005730cdb13a51a8416340638e9/components/policy/core/common/registry_dict.h
[rename] https://crrev.com/32d00fec960c4005730cdb13a51a8416340638e9/components/policy/core/common/registry_dict_unittest.cc

Status: Started (was: Assigned)

Comment 7 by tnagel@chromium.org, Nov 21 2016

Labels: Vnone CodeHealth
Labels: -Vnone V1
Moving to V1, we should be able to reuse unit tests if this is fixed.

Stretch goal, though. We could also just copy the unit tests over for now.
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 10 2017

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

commit f601211275a7ed8a187d49099ea3c81c4168b445
Author: ljusten <ljusten@chromium.org>
Date: Tue Jan 10 13:02:43 2017

Push preg_parser changes upstream

preg_parser and registry_dict are used in Chromium OS by authpolicy
code to fetch policy from AD servers. Right now, a custom version of
the two files is checked into platform2 in Chromium OS. This CL
pushes changes made to these files upstream to Chromium, see CrOS
CL:416068. The goal is to eventually use them in libchrome, so that
only one version of the code has to be maintained and unit tests can
be reused.

BUG= chromium:659645 
TEST=Compiled on Linux

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

[modify] https://crrev.com/f601211275a7ed8a187d49099ea3c81c4168b445/components/policy/core/common/preg_parser.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/external/libchrome/+/6430a2797e1dfb3af1b83a17640150d216037698

commit 6430a2797e1dfb3af1b83a17640150d216037698
Author: Lutz Justen <ljusten@chromium.org>
Date: Wed Jan 11 12:30:16 2017

libchrome: Add preg_parser and registry_dict

preg_parser and registry_dict are used by authpolicy to fetch policy
from AD servers. Right now, a custom version of the two files is
checked into platform2 in Chromium OS. This CL puts the Chromium copy
of these files into libchrome, so that authpolicy can reuse this code.

Revision in chromium/src of added files:
f601211275a7ed8a187d49099ea3c81c4168b445
Jan 10 2017, Chrome revision 57.0.2978.0, r442564

BUG= chromium:659645 
TEST=Compiles, policy lib is accessible by authpolicy code.

Change-Id: I221a73b50f58673369a5d1c28a1a742b28ed67a6
Reviewed-on: https://chromium-review.googlesource.com/426860
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/components/policy/core/common/registry_dict.cc
[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/components/policy/core/common/preg_parser.cc
[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/components/policy/core/common/registry_dict.h
[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/components/policy/core/common/preg_parser_unittest.cc
[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/components/policy/core/common/registry_dict_unittest.cc
[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/PRESUBMIT.cfg
[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/components/policy/core/common/preg_parser.h
[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/components/policy/core/common/policy_types.h
[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/components/policy/policy_export.h
[modify] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/SConstruct
[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/components/policy/core/common/policy_load_status.h
[add] https://crrev.com/6430a2797e1dfb3af1b83a17640150d216037698/components/policy/core/common/policy_load_status.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/537bf1d0bd6b90a6cfca99ccf34ea6e349fb6df9

commit 537bf1d0bd6b90a6cfca99ccf34ea6e349fb6df9
Author: Lutz Justen <ljusten@chromium.org>
Date: Wed Jan 11 12:50:10 2017

libchrome: Uprev libchrome and add policy headers

Adds the following commits:
0393f0e Add preg_parser and registry_dict
609cb79 Remove remaining references to scoped_ptr

This CL is a requirement for using preg_parser and registry_dict in
the authpolicy project.

CQ-DEPEND=CL:426860

BUG= chromium:659645 
TEST=Compiles, policy lib is accessible by authpolicy code.

Change-Id: I5617b69265752228ab7e544ec35b389ee0b7bfe5
Reviewed-on: https://chromium-review.googlesource.com/427159
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>

[rename] https://crrev.com/537bf1d0bd6b90a6cfca99ccf34ea6e349fb6df9/chromeos-base/libchrome/libchrome-395517-r5.ebuild
[modify] https://crrev.com/537bf1d0bd6b90a6cfca99ccf34ea6e349fb6df9/chromeos-base/libchrome/libchrome-395517.ebuild

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/d04f651c3c29071e14fee66a67e103644a96fa7d

commit d04f651c3c29071e14fee66a67e103644a96fa7d
Author: Lutz Justen <ljusten@chromium.org>
Date: Wed Jan 11 13:04:05 2017

authpolicy: Use preg_parser and registry_dict from libchrome

Removes the local copy of the two classes and uses the libchrome
version instead. This fixes the double-maintainance problem.

As a nice side-effect, histogram data is recorded if preg_parser fails.
This code was removed from the authpolicy-copy of preg_parser.

CQ-DEPEND=CL:427159

BUG= chromium:659645 
TEST=Emerges, works fine with custom test code

Change-Id: Ibbe335453a4f812899a5d9246729f7a223a8e505
Reviewed-on: https://chromium-review.googlesource.com/427119
Commit-Ready: Lutz Justen <ljusten@chromium.org>
Tested-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>

[modify] https://crrev.com/d04f651c3c29071e14fee66a67e103644a96fa7d/authpolicy/policy/device_policy_encoder.cc
[modify] https://crrev.com/d04f651c3c29071e14fee66a67e103644a96fa7d/authpolicy/policy/preg_policy_encoder.cc
[delete] https://crrev.com/86bc44835b9685a690c06c3cd1dc3be63313cfe4/authpolicy/policy/registry_dict.h
[modify] https://crrev.com/d04f651c3c29071e14fee66a67e103644a96fa7d/authpolicy/authpolicy.gyp
[delete] https://crrev.com/86bc44835b9685a690c06c3cd1dc3be63313cfe4/authpolicy/policy/preg_parser.h
[delete] https://crrev.com/86bc44835b9685a690c06c3cd1dc3be63313cfe4/authpolicy/policy/preg_parser.cc
[modify] https://crrev.com/d04f651c3c29071e14fee66a67e103644a96fa7d/authpolicy/policy/user_policy_encoder.cc
[modify] https://crrev.com/d04f651c3c29071e14fee66a67e103644a96fa7d/authpolicy/policy/user_policy_encoder.h
[delete] https://crrev.com/86bc44835b9685a690c06c3cd1dc3be63313cfe4/authpolicy/policy/registry_dict.cc
[modify] https://crrev.com/d04f651c3c29071e14fee66a67e103644a96fa7d/authpolicy/policy/policy_encoder_helper.cc

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
bulk Verify of Chromad V1 bugs

Sign in to add a comment