Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.
/ jdk19 Public archive

8287971: Throw exception for missing values in .jpackage.xml #9

Closed
wants to merge 4 commits into from
Closed

8287971: Throw exception for missing values in .jpackage.xml #9

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
daee617
8287971: Throw exception for missing values in .jpackage.xml
sashamatveev Jun 13, 2022
c06af06
8287971: Throw exception for missing values in .jpackage.xml [v2]
sashamatveev Jun 16, 2022
2199f4e
8287971: Throw exception for missing values in .jpackage.xml [v3]
sashamatveev Jun 17, 2022
d489ee6
8287971: Throw exception for missing values in .jpackage.xml [v4]
sashamatveev Jun 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 3 additions & 8 deletions src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacBaseInstallerBundler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,14 +140,9 @@ protected void validateAppImageAndBundeler(
SIGN_BUNDLE.fetchFrom(params)).orElse(Boolean.FALSE)) {
// if signing bundle with app-image, warn user if app-image
// is not already signed.
try {
if (!(AppImageFile.load(applicationImage).isSigned())) {
Log.info(MessageFormat.format(I18N.getString(
"warning.unsigned.app.image"), getID()));
}
} catch (IOException ioe) {
// Ignore - In case of a forign or tampered with app-image,
// user is notified of this when the name is extracted.
if (!(AppImageFile.load(applicationImage).isSigned())) {
Log.info(MessageFormat.format(I18N.getString(
"warning.unsigned.app.image"), getID()));
}
}
} else {
Expand Down
131 changes: 54 additions & 77 deletions src/jdk.jpackage/share/classes/jdk/jpackage/internal/AppImageFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,47 @@ public final class AppImageFile {
Platform.LINUX, "linux", Platform.WINDOWS, "windows", Platform.MAC,
"macOS");


private AppImageFile() {
this(null, null, null, null, null, null, null);
}

private AppImageFile(String launcherName, String mainClass,
private AppImageFile(Path appImageDir, String launcherName, String mainClass,
List<LauncherInfo> launcherInfos, String creatorVersion,
String creatorPlatform, String signedStr, String appStoreStr) {
boolean isValid = true;
if (!Objects.equals(getVersion(), creatorVersion)) {
isValid = false;
}

if (!Objects.equals(getPlatform(), creatorPlatform)) {
isValid = false;
}

if (launcherName == null || launcherName.length() == 0) {
isValid = false;
}

if (mainClass == null || mainClass.length() == 0) {
isValid = false;
}

for (var launcher : launcherInfos) {
if ("".equals(launcher.getName())) {
isValid = false;
}
}

if (signedStr == null ||
!("true".equals(signedStr) || "false".equals(signedStr))) {
isValid = false;
}

if (appStoreStr == null ||
!("true".equals(appStoreStr) || "false".equals(appStoreStr))) {
isValid = false;
}

if (!isValid) {
throw new RuntimeException(MessageFormat.format(I18N.getString(
"error.invalid-app-image"), appImageDir));
}

this.launcherName = launcherName;
this.mainClass = mainClass;
this.addLauncherInfos = launcherInfos;
Expand Down Expand Up @@ -121,10 +154,6 @@ boolean isAppStore() {
return appStore;
}

void verifyCompatible() throws ConfigException {
// Just do nothing for now.
}

/**
* Returns path to application image info file.
* @param appImageDir - path to application image
Expand Down Expand Up @@ -189,25 +218,17 @@ static void save(Path appImageDir, Map<String, Object> params)
* @return valid info about application image or null
* @throws IOException
*/
static AppImageFile load(Path appImageDir) throws IOException {
static AppImageFile load(Path appImageDir) {
try {
Document doc = readXml(appImageDir);

XPath xPath = XPathFactory.newInstance().newXPath();

String mainLauncher = xpathQueryNullable(xPath,
"/jpackage-state/main-launcher/text()", doc);
if (mainLauncher == null) {
// No main launcher, this is fatal.
return new AppImageFile();
}

String mainClass = xpathQueryNullable(xPath,
"/jpackage-state/main-class/text()", doc);
if (mainClass == null) {
// No main class, this is fatal.
return new AppImageFile();
}

List<LauncherInfo> launcherInfos = new ArrayList<>();

Expand All @@ -231,15 +252,17 @@ static AppImageFile load(Path appImageDir) throws IOException {
launcherInfos.add(new LauncherInfo(launcherNodes.item(i)));
}

AppImageFile file = new AppImageFile(mainLauncher, mainClass,
return new AppImageFile(appImageDir, mainLauncher, mainClass,
launcherInfos, version, platform, signedStr, appStoreStr);
if (!file.isValid()) {
file = new AppImageFile();
}
return file;
} catch (XPathExpressionException ex) {
// This should never happen as XPath expressions should be correct
throw new RuntimeException(ex);
} catch (NoSuchFileException nsfe) {
// non jpackage generated app-image (no app/.jpackage.xml)
throw new RuntimeException(MessageFormat.format(I18N.getString(
"error.foreign-app-image"), appImageDir));
} catch (IOException ioe) {
throw new RuntimeException(ioe);
}
}

Expand Down Expand Up @@ -275,23 +298,11 @@ static List<LauncherInfo> getLaunchers(Path appImageDir,
Map<String, Object> params) {
List<LauncherInfo> launchers = new ArrayList<>();
if (appImageDir != null) {
try {
AppImageFile appImageInfo = AppImageFile.load(appImageDir);
if (appImageInfo != null) {
launchers.add(new LauncherInfo(
appImageInfo.getLauncherName(), params));
AppImageFile appImageInfo = AppImageFile.load(appImageDir);
launchers.add(new LauncherInfo(
appImageInfo.getLauncherName(), params));
launchers.addAll(appImageInfo.getAddLaunchers());
return launchers;
}
} catch (NoSuchFileException nsfe) {
// non jpackage generated app-image (no app/.jpackage.xml)
Log.info(MessageFormat.format(I18N.getString(
"warning.foreign-app-image"), appImageDir));
} catch (IOException ioe) {
Log.verbose(ioe);
Log.info(MessageFormat.format(I18N.getString(
"warning.invalid-app-image"), appImageDir));
}
return launchers;
}

launchers.add(new LauncherInfo(params));
Expand All @@ -302,23 +313,11 @@ static List<LauncherInfo> getLaunchers(Path appImageDir,
}

public static String extractAppName(Path appImageDir) {
try {
return AppImageFile.load(appImageDir).getLauncherName();
} catch (IOException ioe) {
Log.verbose(MessageFormat.format(I18N.getString(
"warning.foreign-app-image"), appImageDir));
return null;
}
return AppImageFile.load(appImageDir).getLauncherName();
}

public static String extractMainClass(Path appImageDir) {
try {
return AppImageFile.load(appImageDir).getMainClass();
} catch (IOException ioe) {
Log.verbose(MessageFormat.format(I18N.getString(
"warning.foreign-app-image"), appImageDir));
return null;
}
return AppImageFile.load(appImageDir).getMainClass();
}

private static String xpathQueryNullable(XPath xPath, String xpathExpr,
Expand All @@ -332,35 +331,13 @@ private static String xpathQueryNullable(XPath xPath, String xpathExpr,
}

static String getVersion() {
return "1.0";
return System.getProperty("java.version");
}

static String getPlatform() {
return PLATFORM_LABELS.get(Platform.getPlatform());
}

private boolean isValid() {
if (launcherName == null || launcherName.length() == 0) {
return false;
}

for (var launcher : addLauncherInfos) {
if ("".equals(launcher.getName())) {
return false;
}
}

if (!Objects.equals(getVersion(), creatorVersion)) {
return false;
}

if (!Objects.equals(getPlatform(), creatorPlatform)) {
return false;
}

return true;
}

static class LauncherInfo {
private final String name;
private final boolean shortcut;
Expand Down
8 changes: 2 additions & 6 deletions src/jdk.jpackage/share/classes/jdk/jpackage/internal/StandardBundlerParam.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,12 +531,8 @@ private static Path findPathOfModule( List<Path> modulePath, String moduleName)
Boolean.class,
params -> {
if (hasPredefinedAppImage(params)) {
try {
return AppImageFile.load(getPredefinedAppImage(params))
.isAppStore();
} catch (IOException ex) {
return false;
}
return AppImageFile.load(getPredefinedAppImage(params))
.isAppStore();
}
return false;
},
Expand Down
5 changes: 3 additions & 2 deletions src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,9 @@ error.blocked.option=jlink option [{0}] is not permitted in --jlink-options
error.no.name=Name not specified with --name and cannot infer one from app-image

warning.no.jdk.modules.found=Warning: No JDK Modules found
warning.foreign-app-image=Warning: app-image dir ({0}) not generated by jpackage.
warning.invalid-app-image=Warning: cannot parse .jpackage.xml in app-image dir ({0})

error.foreign-app-image=Error: Missing .jpackage.xml file in app-image dir ({0})
error.invalid-app-image=Error: app-image dir ({0}) generated by another jpackage version or malformed .jpackage.xml

MSG_BundlerFailed=Error: Bundler "{1}" ({0}) failed to produce a package
MSG_BundlerConfigException=Bundler {0} skipped because of a configuration problem: {1} \n\
Expand Down
5 changes: 3 additions & 2 deletions src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_de.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ error.blocked.option=jlink-Option [{0}] ist in --jlink-options nicht zul\u00E4ss
error.no.name=Name nicht mit --name angegeben. Es kann auch kein Name aus app-image abgeleitet werden

warning.no.jdk.modules.found=Warnung: Keine JDK-Module gefunden
warning.foreign-app-image=Warnung: app-image-Verzeichnis ({0}) wurde von jpackage nicht generiert.
warning.invalid-app-image=Warnung: .jpackage.xml kann in app-image-Verzeichnis ({0}) nicht geparst werden

error.foreign-app-image=Error: Missing .jpackage.xml file in app-image dir ({0})
error.invalid-app-image=Error: app-image dir ({0}) generated by another jpackage version or malformed .jpackage.xml

MSG_BundlerFailed=Fehler: Bundler "{1}" ({0}) konnte kein Package generieren
MSG_BundlerConfigException=Bundler {0} aufgrund eines Konfigurationsproblems \u00FCbersprungen: {1} \nEmpfehlung zur Behebung: {2}
Expand Down
5 changes: 3 additions & 2 deletions src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_ja.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ error.blocked.option=jlink\u30AA\u30D7\u30B7\u30E7\u30F3[{0}]\u306F--jlink-optio
error.no.name=\u540D\u524D\u304C--name\u3067\u6307\u5B9A\u3055\u308C\u3066\u304A\u3089\u305A\u3001app-image\u304B\u3089\u63A8\u8AD6\u3067\u304D\u307E\u305B\u3093

warning.no.jdk.modules.found=\u8B66\u544A: JDK\u30E2\u30B8\u30E5\u30FC\u30EB\u304C\u898B\u3064\u304B\u308A\u307E\u305B\u3093
warning.foreign-app-image=\u8B66\u544A: app-image\u30C7\u30A3\u30EC\u30AF\u30C8\u30EA({0})\u306Fjpackage\u3067\u751F\u6210\u3055\u308C\u307E\u305B\u3093\u3002
warning.invalid-app-image=\u8B66\u544A: app-image\u30C7\u30A3\u30EC\u30AF\u30C8\u30EA({0})\u306E.jpackage.xml\u3092\u89E3\u6790\u3067\u304D\u307E\u305B\u3093

error.foreign-app-image=Error: Missing .jpackage.xml file in app-image dir ({0})
error.invalid-app-image=Error: app-image dir ({0}) generated by another jpackage version or malformed .jpackage.xml

MSG_BundlerFailed=\u30A8\u30E9\u30FC: \u30D0\u30F3\u30C9\u30E9"{1}" ({0})\u304C\u30D1\u30C3\u30B1\u30FC\u30B8\u306E\u751F\u6210\u306B\u5931\u6557\u3057\u307E\u3057\u305F
MSG_BundlerConfigException=\u69CB\u6210\u306E\u554F\u984C\u306E\u305F\u3081\u3001\u30D0\u30F3\u30C9\u30E9{0}\u304C\u30B9\u30AD\u30C3\u30D7\u3055\u308C\u307E\u3057\u305F: {1} \n\u6B21\u306E\u4FEE\u6B63\u3092\u884C\u3063\u3066\u304F\u3060\u3055\u3044: {2}
Expand Down
5 changes: 3 additions & 2 deletions ...jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_zh_CN.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ error.blocked.option=\u4E0D\u5141\u8BB8\u5728 --jlink-options \u4E2D\u4F7F\u7528
error.no.name=\u672A\u4F7F\u7528 --name \u6307\u5B9A\u540D\u79F0\uFF0C\u65E0\u6CD5\u4ECE app-image \u63A8\u65AD\u540D\u79F0

warning.no.jdk.modules.found=\u8B66\u544A: \u672A\u627E\u5230 JDK \u6A21\u5757
warning.foreign-app-image=\u8B66\u544A\uFF1Ajpackage \u672A\u751F\u6210 app-image \u76EE\u5F55 ({0})\u3002
warning.invalid-app-image=\u8B66\u544A\uFF1A\u65E0\u6CD5\u89E3\u6790 app-image \u76EE\u5F55 ({0}) \u4E2D\u7684 .jpackage.xml

error.foreign-app-image=Error: Missing .jpackage.xml file in app-image dir ({0})
error.invalid-app-image=Error: app-image dir ({0}) generated by another jpackage version or malformed .jpackage.xml

MSG_BundlerFailed=\u9519\u8BEF\uFF1A\u6253\u5305\u7A0B\u5E8F "{1}" ({0}) \u65E0\u6CD5\u751F\u6210\u7A0B\u5E8F\u5305
MSG_BundlerConfigException=\u7531\u4E8E\u914D\u7F6E\u95EE\u9898, \u8DF3\u8FC7\u4E86\u6253\u5305\u7A0B\u5E8F{0}: {1} \n\u4FEE\u590D\u5EFA\u8BAE: {2}
Expand Down
52 changes: 52 additions & 0 deletions test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import jdk.jpackage.internal.IOUtils;
import jdk.jpackage.internal.AppImageFile;
import jdk.jpackage.internal.ApplicationLayout;
import jdk.jpackage.internal.PackageFile;
Expand Down Expand Up @@ -298,6 +299,57 @@ public JPackageCommand setFakeRuntime() {
return this;
}

public void createJPackageXMLFile(String mainLauncher, String mainClass)
throws IOException {
String appImage = getArgumentValue("--app-image");
if (appImage == null) {
throw new RuntimeException("Error: --app-image expected");
}
Copy link
Member

Choose a reason for hiding this comment

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

Can be simplified to:

Path jpackageXMLFile = AppImageFile.getPathInAppImage(
                Optional.ofNullable(getArgumentValue("--app-image")).map(
                        Path::of).orElseThrow(() -> {
                            return new RuntimeException(
                                    "Error: --app-image expected");
                        }));

And you don't need copy/paste code from AppImageFile.getPathInAppImage() below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. I also switched to use AppImageFile.getVersion() and AppImageFile.getPlatform().


final String version = System.getProperty("java.version");
final String platform;

if (TKit.isWindows()) {
platform = "windows";
} else if (TKit.isLinux()) {
platform = "linux";
} else if (TKit.isOSX()) {
platform = "macOS";
} else {
platform = "unknown";
}

ApplicationLayout layout = ApplicationLayout.platformAppImage();
Path jpackageXMLFile = layout.resolveAt(Path.of(appImage))
.appDirectory().resolve(".jpackage.xml");

IOUtils.createXml(jpackageXMLFile, xml -> {
xml.writeStartElement("jpackage-state");
xml.writeAttribute("version", version);
xml.writeAttribute("platform", platform);

xml.writeStartElement("app-version");
xml.writeCharacters("1.0");
xml.writeEndElement();

xml.writeStartElement("main-launcher");
xml.writeCharacters(mainLauncher);
xml.writeEndElement();

xml.writeStartElement("main-class");
xml.writeCharacters(mainClass);
xml.writeEndElement();

xml.writeStartElement("signed");
xml.writeCharacters("false");
xml.writeEndElement();

xml.writeStartElement("app-store");
xml.writeCharacters("false");
xml.writeEndElement();
});
}

JPackageCommand addPrerequisiteAction(ThrowingConsumer<JPackageCommand> action) {
verifyMutable();
prerequisiteActions.add(action);
Expand Down