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

Handle osgi headers correctly #54

Open
wants to merge 1 commit into
base: main
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
12 changes: 12 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@
<version>1.10.14</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.eclipse.platform</groupId>
<artifactId>org.eclipse.osgi</artifactId>
<version>3.18.600</version>
</dependency>

<!-- Test dependencies -->
<dependency>
Expand All @@ -103,6 +108,7 @@
<version>4.13.2</version>
<scope>test</scope>
</dependency>

</dependencies>

<profiles>
Expand Down Expand Up @@ -276,6 +282,12 @@
<exclude>META-INF/**</exclude>
</excludes>
</filter>
<filter>
<artifact>org.eclipse.platform:*</artifact>
<excludes>
<exclude>META-INF/**</exclude>
</excludes>
</filter>
</filters>
<relocations>
<relocation>
Expand Down
77 changes: 65 additions & 12 deletions src/main/java/org/apache/tomcat/jakartaee/ManifestConverter.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,25 @@
import java.util.regex.Pattern;

import org.apache.commons.io.IOUtils;
import org.eclipse.osgi.util.ManifestElement;
import org.osgi.framework.BundleException;
import org.osgi.framework.Constants;

/**
* Updates Manifests.
*/
public class ManifestConverter implements Converter {

private static final String JAKARTA_SERVLET = "jakarta.servlet";
private static final Pattern SERVLET_PATTERN = Pattern.compile("jakarta.servlet([^,]*);version=\"(.*?)\"");
private static final Logger logger = Logger.getLogger(ManifestConverter.class.getCanonicalName());
private static final StringManager sm = StringManager.getManager(ManifestConverter.class);

/**
* Manifest converter constructor.
*/
public ManifestConverter() {}
public ManifestConverter() {
}

@Override
public boolean accepts(String filename) {
Expand All @@ -63,7 +69,8 @@ public boolean convert(String path, InputStream src, OutputStream dest, EESpecPr
Manifest srcManifest = new Manifest(new ByteArrayInputStream(srcBytes));
Manifest destManifest = new Manifest(srcManifest);

// Only consider profile conversions, allowing Migration.hasConverted to be true only when there are actual
// Only consider profile conversions, allowing Migration.hasConverted to be true
// only when there are actual
// conversions made
boolean converted = updateValues(destManifest, profile);
removeSignatures(destManifest);
Expand All @@ -80,7 +87,6 @@ public boolean convert(String path, InputStream src, OutputStream dest, EESpecPr
return converted;
}


private void removeSignatures(Manifest manifest) {
manifest.getMainAttributes().remove(Attributes.Name.SIGNATURE_VERSION);
List<String> signatureEntries = new ArrayList<>();
Expand All @@ -98,7 +104,6 @@ private void removeSignatures(Manifest manifest) {
}
}


private boolean isCryptoSignatureEntry(Attributes attributes) {
for (Object attributeKey : attributes.keySet()) {
if (attributeKey.toString().endsWith("-Digest")) {
Expand All @@ -108,7 +113,6 @@ private boolean isCryptoSignatureEntry(Attributes attributes) {
return false;
}


private boolean updateValues(Manifest manifest, EESpecProfile profile) {
boolean converted = updateValues(manifest.getMainAttributes(), profile);
for (Attributes attributes : manifest.getEntries().values()) {
Expand All @@ -117,7 +121,6 @@ private boolean updateValues(Manifest manifest, EESpecProfile profile) {
return converted;
}


private boolean updateValues(Attributes attributes, EESpecProfile profile) {
boolean converted = false;
// Update version info
Expand All @@ -128,24 +131,74 @@ private boolean updateValues(Attributes attributes, EESpecProfile profile) {
// Purposefully avoid setting result
}
// Update package names in values
for (Entry<Object,Object> entry : attributes.entrySet()) {
for (Entry<Object, Object> entry : attributes.entrySet()) {
String newValue = profile.convert((String) entry.getValue());
newValue = replaceVersion(newValue);
String header = entry.getKey().toString();
try {
// Need to be careful with OSGI headers.
// Specifically, Export-Package cannot specify a version range.
// There may be other weird things as well (like directives that have
// jakarta.servlet packages).
if (Constants.IMPORT_PACKAGE.equals(header)) {
newValue = processImportPackage(newValue);
} else if (Constants.EXPORT_PACKAGE.equals(header)) {
newValue = processExportPackage(newValue);
} else {
newValue = replaceVersion(newValue);
}
} catch (BundleException e) {
newValue = replaceVersion(newValue, !Constants.EXPORT_PACKAGE.equals(header));
}

// Object comparison is deliberate
if (newValue != entry.getValue()) {
if (!newValue.equals(entry.getValue())) {
entry.setValue(newValue);
converted = true;
}
}
return converted;
}

private String processExportPackage(String value) throws BundleException {
return processOSGIHeader(value, Constants.EXPORT_PACKAGE, "5.0.0");
}

private String processImportPackage(String value) throws BundleException {
return processOSGIHeader(value, Constants.IMPORT_PACKAGE, "[5.0.0,7.0.0)");
}

private String processOSGIHeader(String value, String header, String replacement) throws BundleException {
List<String> packages = new ArrayList<>();
ManifestElement[] elements = ManifestElement.parseHeader(header, value);
for (ManifestElement element : elements) {
if (element.getValue().startsWith(JAKARTA_SERVLET)) {
String oldVersion = element.getAttribute(Constants.VERSION_ATTRIBUTE);
if (oldVersion != null) {
packages.add(element.toString().replace(oldVersion, replacement));
} else {
packages.add(element.toString());
}
} else {
packages.add(element.toString());
}
}
if (packages.isEmpty()) {
return value;
}
return String.join(",", packages);
}

private String replaceVersion(String entryValue) {
if (entryValue.contains("jakarta.servlet")) {
return replaceVersion(entryValue, true);
}

private String replaceVersion(String entryValue, boolean range) {
if (entryValue.contains(JAKARTA_SERVLET)) {
StringBuffer builder = new StringBuffer();
Matcher matcher = Pattern.compile("jakarta.servlet([^,]*);version=\"(.*?)\"").matcher(entryValue);
Matcher matcher = SERVLET_PATTERN.matcher(entryValue);
while (matcher.find()) {
matcher.appendReplacement(builder, "jakarta.servlet$1;version=\"[5.0.0,7.0.0)\"");
String version = range ? "[5.0.0,7.0.0)" : "5.0.0";
matcher.appendReplacement(builder, "jakarta.servlet$1;version=\"" + version + "\"");
}
matcher.appendTail(builder);
return builder.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,13 @@
package org.apache.tomcat.jakartaee;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;

import org.junit.Test;

public class ManifestConverterTest {
Expand All @@ -34,4 +39,39 @@ public void testAccepts() {
assertFalse(converter.accepts("xMETA-INF/MANIFEST.MF"));
assertFalse(converter.accepts("WEB-INF/bundles/com.example.bundle/xMETA-INF/MANIFEST.MF"));
}

@Test
public void testConvert() throws IOException {
ManifestConverter converter = new ManifestConverter();
ByteArrayOutputStream os = new ByteArrayOutputStream();
boolean converted = converter.convert("/MANIFEST.test.MF", getClass().getResourceAsStream("/MANIFEST.test.MF"), //$NON-NLS-1$
os, EESpecProfiles.TOMCAT);
assertTrue(converted);

String result = os.toString(StandardCharsets.UTF_8);
assertTrue(result.length() != 0);
result = result.replaceAll("\\s", "");

// Basic test
String imports = "jakarta.servlet;version=\"[5.0.0,7.0.0)\"";

// Test with directives
String imports2 = "jakarta.servlet.http;version=\"[5.0.0,7.0.0)\";resolution:=\"optional\"";
assertTrue(result.contains(imports));
assertTrue(result.contains(imports2));

// Test with directive and version
String exports = "jakarta.servlet;version=\"5.0.0\";uses:=\"org.eclipse.core.runtime\"";

// Same as above, with javax.servlet package in the directive
String exports2 = "jakarta.servlet.http;version=\"5.0.0\";uses:=\"jakarta.servlet\"";

// Export a different package that has javax.servlet in a directive so version
// isn't updated
String exports3 = "org.apache.tomcat.jakartaee.test;version=\"1.0.0\";uses:=\"jakarta.servlet\"";

assertTrue(result.contains(exports));
assertTrue(result.contains(exports2));
assertTrue(result.contains(exports3));
}
}
9 changes: 9 additions & 0 deletions src/test/resources/MANIFEST.test.MF
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Version: 1.0.0.qualifier
Bundle-SymbolicName: org.apache.tomcat.jakartaee.test
Import-Package: javax.servlet;version="[2.0.0,5.0.0)",
javax.servlet.http;resolution:=optional;version="[2.0.0,5.0.0)"
Export-Package: javax.servlet;uses:="org.eclipse.core.runtime";version="4.0.0",
javax.servlet.http;uses:="javax.servlet";version="4.0.0",
org.apache.tomcat.jakartaee.test;uses:="javax.servlet";version="1.0.0"