Skip to content

[MSHADE-156] - Allow non relocating string literals #249

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
26 changes: 16 additions & 10 deletions src/main/java/org/apache/maven/plugins/shade/DefaultShader.java
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ private void shadeJarEntry(
int method)
throws Exception {
try (InputStream in = inputProvider.call()) {
String mappedName = packageMapper.map(name, true, false);
String mappedName = packageMapper.map(name, true, false, false);

int idx = mappedName.lastIndexOf('/');
if (idx != -1) {
Expand Down Expand Up @@ -619,7 +619,7 @@ private void addRemappedClass(
}

// Need to take the .class off for remapping evaluation
String mappedName = packageMapper.map(name.substring(0, name.indexOf('.')), true, false);
String mappedName = packageMapper.map(name.substring(0, name.indexOf('.')), true, false, false);

try {
// Now we put it back on so the class file is written out with the right extension.
Expand Down Expand Up @@ -729,9 +729,10 @@ private interface PackageMapper {
* @param entityName entity name to be mapped
* @param mapPaths map "slashy" names like paths or internal Java class names, e.g. {@code com/acme/Foo}?
* @param mapPackages map "dotty" names like qualified Java class or package names, e.g. {@code com.acme.Foo}?
* @param mapLiterals map string literals, e.g. {@code "com.acme.Foo"}?
* @return mapped entity name, e.g. {@code org/apache/acme/Foo} or {@code org.apache.acme.Foo}
*/
String map(String entityName, boolean mapPaths, boolean mapPackages);
String map(String entityName, boolean mapPaths, boolean mapPackages, boolean mapLiterals);
}

/**
Expand All @@ -747,7 +748,7 @@ private DefaultPackageMapper(final List<Relocator> relocators) {
}

@Override
public String map(String entityName, boolean mapPaths, final boolean mapPackages) {
public String map(String entityName, boolean mapPaths, final boolean mapPackages, final boolean mapLiterals) {
String value = entityName;

String prefix = "";
Expand All @@ -761,7 +762,9 @@ public String map(String entityName, boolean mapPaths, final boolean mapPackages
}

for (Relocator r : relocators) {
if (mapPackages && r.canRelocateClass(entityName)) {
if (mapLiterals && r.skipStringLiteral()) {
continue;
} else if (mapPackages && r.canRelocateClass(entityName)) {
value = prefix + r.relocateClass(entityName) + suffix;
break;
} else if (mapPaths && r.canRelocatePath(entityName)) {
Expand All @@ -778,7 +781,9 @@ private static class LazyInitRemapper extends Remapper {

@Override
public Object mapValue(Object object) {
return object instanceof String ? relocators.map((String) object, true, true) : super.mapValue(object);
return object instanceof String
? relocators.map((String) object, true, true, true)
: super.mapValue(object);
}

@Override
Expand All @@ -791,7 +796,7 @@ public String map(String name) {
// TODO: Analyse if this case is really necessary and has any special meaning or avoids any known problems.
// If not, then simplify DefaultShader.PackageMapper.map to only have the String parameter and assume
// both boolean ones to always be true.
return relocators.map(name, true, false);
return relocators.map(name, true, false, false);
}
}

Expand Down Expand Up @@ -828,14 +833,15 @@ public void visitSource(final String source, final String debug) {
}

final String fqSource = pkg + source;
final String mappedSource = map(fqSource, true, false);
final String mappedSource = map(fqSource, true, false, false);
final String filename = mappedSource.substring(mappedSource.lastIndexOf('/') + 1);
super.visitSource(filename, debug);
}

@Override
public String map(final String entityName, boolean mapPaths, final boolean mapPackages) {
final String mapped = packageMapper.map(entityName, true, mapPackages);
public String map(
final String entityName, boolean mapPaths, final boolean mapPackages, final boolean mapLiterals) {
final String mapped = packageMapper.map(entityName, true, mapPackages, mapLiterals);
if (!remapped) {
remapped = !mapped.equals(entityName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public class PackageRelocation {

private boolean rawString;

private boolean skipStringLiteral;

public String getPattern() {
return pattern;
}
Expand All @@ -54,4 +56,8 @@ public List<String> getExcludes() {
public boolean isRawString() {
return rawString;
}

public boolean isSkipStringLiteral() {
return skipStringLiteral;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,12 @@ private List<Relocator> getRelocators() {

for (PackageRelocation r : relocations) {
relocators.add(new SimpleRelocator(
r.getPattern(), r.getShadedPattern(), r.getIncludes(), r.getExcludes(), r.isRawString()));
r.getPattern(),
r.getShadedPattern(),
r.getIncludes(),
r.getExcludes(),
r.isRawString(),
r.isSkipStringLiteral()));
}

return relocators;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,6 @@ public interface Relocator {
String relocateClass(String clazz);

String applyToSourceContent(String sourceContent);

boolean skipStringLiteral();
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,21 @@ public class SimpleRelocator implements Relocator {

private final boolean rawString;

private final boolean skipStringLiteral;

public SimpleRelocator(String patt, String shadedPattern, List<String> includes, List<String> excludes) {
this(patt, shadedPattern, includes, excludes, false);
this(patt, shadedPattern, includes, excludes, false, false);
}

public SimpleRelocator(
String patt, String shadedPattern, List<String> includes, List<String> excludes, boolean rawString) {
String patt,
String shadedPattern,
List<String> includes,
List<String> excludes,
boolean rawString,
boolean skipStringLiteral) {
this.rawString = rawString;
this.skipStringLiteral = skipStringLiteral;

if (rawString) {
this.pathPattern = patt;
Expand Down Expand Up @@ -257,4 +265,8 @@ private String shadeSourceWithExcludes(
}
return shadedSourceContent.toString();
}

public boolean skipStringLiteral() {
return skipStringLiteral;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,45 @@ public void testShaderWithStaticInitializedClass() throws Exception {
}
}

@Test
public void testShaderWithoutStringLiterals() throws Exception {
Shader s = newShader();

Set<File> set = new LinkedHashSet<>();

set.add(new File("src/test/jars/test-artifact-1.0-SNAPSHOT.jar"));

List<Relocator> relocators = new ArrayList<>();

relocators.add(new SimpleRelocator("org.apache.maven.plugins.shade", null, null, null, false, false));
relocators.add(new SimpleRelocator("foo.bar", null, null, null, false, true));
relocators.add(new SimpleRelocator("org.codehaus", null, null, null, false, true));

List<ResourceTransformer> resourceTransformers = new ArrayList<>();

List<Filter> filters = new ArrayList<>();

File file = new File("target/testShaderWithoutStringLiterals.jar");

ShadeRequest shadeRequest = new ShadeRequest();
shadeRequest.setJars(set);
shadeRequest.setUberJar(file);
shadeRequest.setFilters(filters);
shadeRequest.setRelocators(relocators);
shadeRequest.setResourceTransformers(resourceTransformers);

s.shade(shadeRequest);

try (URLClassLoader cl = new URLClassLoader(new URL[] {file.toURI().toURL()})) {
Class<?> c = cl.loadClass("hidden.org.apache.maven.plugins.shade.Lib");
Object o = c.newInstance();
assertEquals("foo.bar/baz", c.getDeclaredField("CONSTANT").get(o));
assertEquals(
"org.codehaus.plexus.util.xml.pull",
c.getDeclaredField("CLASS_REALM_PACKAGE_IMPORT").get(o));
}
}

@Test
public void testShaderWithCustomShadedPattern() throws Exception {
shaderWithPattern("org/shaded/plexus/util", new File("target/foo-custom.jar"), EXCLUDES);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class SimpleRelocatorTest {

@Test
public void testNoNpeRelocateClass() {
new SimpleRelocator("foo", "bar", null, null, true).relocateClass("foo");
new SimpleRelocator("foo", "bar", null, null, true, false).relocateClass("foo");
}

@Test
Expand Down Expand Up @@ -100,10 +100,10 @@ public void testCanRelocateClass() {
public void testCanRelocateRawString() {
SimpleRelocator relocator;

relocator = new SimpleRelocator("org/foo", null, null, null, true);
relocator = new SimpleRelocator("org/foo", null, null, null, true, false);
assertTrue(relocator.canRelocatePath("(I)org/foo/bar/Class;"));

relocator = new SimpleRelocator("^META-INF/org.foo.xml$", null, null, null, true);
relocator = new SimpleRelocator("^META-INF/org.foo.xml$", null, null, null, true, false);
assertTrue(relocator.canRelocatePath("META-INF/org.foo.xml"));
}

Expand Down Expand Up @@ -161,10 +161,11 @@ public void testRelocateClass() {
public void testRelocateRawString() {
SimpleRelocator relocator;

relocator = new SimpleRelocator("Lorg/foo", "Lhidden/org/foo", null, null, true);
relocator = new SimpleRelocator("Lorg/foo", "Lhidden/org/foo", null, null, true, false);
assertEquals("(I)Lhidden/org/foo/bar/Class;", relocator.relocatePath("(I)Lorg/foo/bar/Class;"));

relocator = new SimpleRelocator("^META-INF/org.foo.xml$", "META-INF/hidden.org.foo.xml", null, null, true);
relocator =
new SimpleRelocator("^META-INF/org.foo.xml$", "META-INF/hidden.org.foo.xml", null, null, true, false);
assertEquals("META-INF/hidden.org.foo.xml", relocator.relocatePath("META-INF/org.foo.xml"));
}

Expand Down Expand Up @@ -256,7 +257,8 @@ public void testRelocateSourceWithExcludesRaw() {
"com.acme.maven",
Arrays.asList("foo.bar", "zot.baz"),
Arrays.asList("irrelevant.exclude", "org.apache.maven.exclude1", "org.apache.maven.sub.exclude2"),
true);
true,
false);
assertEquals(sourceFile, relocator.applyToSourceContent(sourceFile));
}

Expand Down