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

JCRVLT-767: vlt: potential incorrect identifier comparison #348

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/site/markdown/packagetypes.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Container Package

Container packages act as deployment vehicle and don't contain regular nodes. Only OSGi bundles, configuration and sub packages (for use with the [OSGi Installer](https://sling.apache.org/documentation/bundles/jcr-installer-provider.html)) are allowed. In addition also sub packages below `/etc/packages/...` are supported.

Containers may be nested which means they may contain itself packages of type `container`.
Containers may be nested which means they may contain themselves packages of type `container`.


Mixed Package
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -989,8 +989,17 @@ private void removeReferences(@NotNull Node node) throws ReferentialIntegrityExc
VersioningState vs = new VersioningState(stack, node);
Node updatedNode = null;
Optional<String> identifier = ni.getIdentifier();

String nodeUUID = null;
try {
nodeUUID = node.getUUID();
reschke marked this conversation as resolved.
Show resolved Hide resolved
} catch (RepositoryException ex) {
// node not referenceable
}

// try to set uuid via sysview import if it differs from existing one
if (identifier.isPresent() && !node.getIdentifier().equals(identifier.get()) && !"rep:root".equals(ni.getPrimaryType().orElse(""))) {
if (identifier.isPresent() && nodeUUID != null && !nodeUUID.equals(identifier.get())
&& !"rep:root".equals(ni.getPrimaryType().orElse(""))) {
long startTime = System.currentTimeMillis();
String previousIdentifier = node.getIdentifier();
log.debug("Node stashing for {} starting, existing identifier: {}, new identifier: {}, import mode: {}",
Expand Down Expand Up @@ -1047,8 +1056,38 @@ private void removeReferences(@NotNull Node node) throws ReferentialIntegrityExc
}
// add remaining mixins (for all import modes)
for (String mixin : newMixins) {
Property uuidProp = null;
vs.ensureCheckedOut();
node.addMixin(mixin);

if (mixin.equals("mix:referenceable") && identifier.isPresent()) {
// if this is mix:ref and ni specifies an identifier,
// try to set it
try {
uuidProp = node.setProperty(Property.JCR_UUID, identifier.get());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't the uuid already set through the sysview import?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we get here in case we skipped that...?

} catch (RepositoryException ex) {
log.debug("tried to set jcr:uuid to {} before adding mix:referenceable failed, continuing", ex);
}

try {
node.addMixin(mixin);
} catch (RepositoryException ex) {
// try to add the mixin; this fails with classic
// jackrabbit, so undo the change for jcr:uuid and retry
if (uuidProp != null) {
log.debug(
"failed to add mix:referenceable after setting jcr:uuid, retrying without (uuid will be lost)",
ex);
// retry once after removing jcr:uuid
uuidProp.remove();
node.addMixin(mixin);
} else {
throw ex;
}
}
} else {
node.addMixin(mixin);
}

updatedNode = node;
}
}
Expand Down