Skip to content

Commit

Permalink
revert: TRUNK-6188: Add whitelisting for components loaded via Xstream
Browse files Browse the repository at this point in the history
  • Loading branch information
mogoodrich committed Sep 3, 2024
1 parent 09b4f6c commit 6db57c4
Showing 1 changed file with 0 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ public Object unmarshal(HierarchicalStreamReader reader, Object root) {
xstream.registerConverter(new IndicatorConverter(mapper, converterLookup));

xstream.registerConverter(new ReportDefinitionConverter(mapper, converterLookup));

setupXStreamSecurity(xstream);
}

@Override
Expand Down Expand Up @@ -117,22 +115,4 @@ public void serializeToStream(Object object, OutputStream out) {
throw new IllegalStateException("Unsupported encoding", e);
}
}

private void setupXStreamSecurity(XStream xstream) throws SerializationException {
try {
SimpleXStreamSerializer serializer = Context.getRegisteredComponent("simpleXStreamSerializer", SimpleXStreamSerializer.class);
if (serializer != null) {
try {
Method method = serializer.getClass().getMethod("initXStream", XStream.class);
method.invoke(serializer, xstream);
}
catch (Exception ex) {
throw new SerializationException("Failed to set up XStream Security", ex);
}
}
}
catch (APIException ex) {
//Ignore APIException("Error during getting registered component) for platform versions below 2.7.0
}
}
}

8 comments on commit 6db57c4

@wikumChamith
Copy link
Member

Choose a reason for hiding this comment

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

@mogoodrich this change is causing an error on the Reference Application 2.x when viewing a patient. Here is the ticket for that issue: https://openmrs.atlassian.net/browse/RA-2058

cc: @dkayiwa, @ibacher

@mogoodrich
Copy link
Member Author

Choose a reason for hiding this comment

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

@wikumChamith looks like this was just a revert of a partial change made here: https://openmrs.atlassian.net/browse/TRUNK-6188

Not sure where we are with the above?

@wikumChamith
Copy link
Member

Choose a reason for hiding this comment

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

Can't we add the whitelisting again?

@mogoodrich
Copy link
Member Author

Choose a reason for hiding this comment

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

Thoughts on @wikumChamith question @ibacher @rkorytkowski @dkayiwa ? (See my commentary on TRUNK-6188 for more details)

@wikumChamith
Copy link
Member

Choose a reason for hiding this comment

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

@mogoodrich I was able to reproduce the issue you encountered on OpenMRS Core 2.6.9 . However, it seems this issue has been resolved in OpenMRS Core 2.7

@mogoodrich
Copy link
Member Author

Choose a reason for hiding this comment

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

@wikumChamith ah!, Could we just apply this change in Core 2.7.0 then?

@wikumChamith
Copy link
Member

@wikumChamith wikumChamith commented on 6db57c4 Nov 20, 2024

Choose a reason for hiding this comment

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

We could enable whitelisting to this module after backporting this commit to the 2.6.x.

Here is the PR for the backporting: openmrs/openmrs-core#4839

@dkayiwa
Copy link
Member

Choose a reason for hiding this comment

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

I would not back port that new feature.

Please sign in to comment.