[
https://issues.apache.org/jira/browse/JCRVLT-683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17719719#comment-17719719
]
Thomas Mueller edited comment on JCRVLT-683 at 5/5/23 7:49 AM:
---------------------------------------------------------------
> Every code relying on that bug can be easily fixed by adjusting the
> aclHandling.
In general, I think that most bugfixes should come with "circuit breakers"
(feature flags) that allow people to disable the "bugfix", specially if the
bugfix changes behavior. In the past, we have used system properties to allow
switching back to the old behavior.
(There are bugfixes that _don't_ change the behavior, for example if they only
improve performance.)
I also think that most changes should be reviewed. It seems the linked PRs
don't currently have reviewers; is think the review process is done in some
other way? Update: sorry,
https://github.com/apache/jackrabbit-filevault/pull/272 doesn't have reviewers;
https://github.com/apache/jackrabbit-filevault/pull/294 has reviewers.
PRs should also come with test cases. Is it true that
https://github.com/apache/jackrabbit-filevault/pull/294/files doesn't have any
test case? There are a lot of code changes as far as I see.
was (Author: tmueller):
> Every code relying on that bug can be easily fixed by adjusting the
> aclHandling.
In general, I think that most bugfixes should come with "circuit breakers"
(feature flags) that allow people to disable the "bugfix", specially if the
bugfix changes behavior. In the past, we have used system properties to allow
switching back to the old behavior.
(There are bugfixes that _don't_ change the behavior, for example if they only
improve performance.)
I also think that most changes should be reviewed. It seems the linked PRs
don't currently have reviewers; is think the review process is done in some
other way?
PRs should also come with test cases. Is it true that
https://github.com/apache/jackrabbit-filevault/pull/294/files doesn't have any
test case? There are a lot of code changes as far as I see.
> Import of Authorizable node with acHandling=IGNORE should preserve existing
> rep:principalPolicy child node
> ----------------------------------------------------------------------------------------------------------
>
> Key: JCRVLT-683
> URL: https://issues.apache.org/jira/browse/JCRVLT-683
> Project: Jackrabbit FileVault
> Issue Type: Bug
> Components: Packaging
> Affects Versions: 3.6.6
> Reporter: Mark Adamcin
> Assignee: Konrad Windszus
> Priority: Major
> Fix For: 3.6.10
>
>
> For situations where an authorizable node may be distributed from another
> environment where a different rep:principalPolicy for the user is defined
> than exists for that user in the target environment, it is important that the
> existing rep:principalPolicy be preserved when acHandling is unset,
> acHandling=IGNORE, or acHandling=MERGE_PRESERVE.
> Currently, the effective behavior of such a package install, as [it appears
> to be implemented in
> DocViewImporter|https://github.com/apache/jackrabbit-filevault/blob/5f9657374bd6c2d3dd1f6e9e2be0b9f5b25ddc26/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java#L782-L787],
> results in the following:
> * If the package specifies acHandling=IGNORE, the existing
> rep:principalPolicy is deleted without replacement, regardless of whether the
> package contains its own rep:principalPolicy, which is equivalent to
> *acHandling=CLEAR*
> * If the package specifies acHandling=MERGE_PRESERVE or MERGE, the existing
> rep:principalPolicy is replaced with whatever rep:principalPolicy is
> contained in the package, or deletes the policy if a replacement is not
> present, which is equivalent to *acHandling=OVERWRITE*
> Unexpectedly, the least destructive (and most default) acHandling mode
> (IGNORE) turns out to be as destructive to packaged system user permissions
> as choosing any other mode.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
