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.

Sign up for GitHub

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

Jump to bottom

Task #3711 - jre of the properties file does not work under windows #3

Merged
merged 4 commits into from
Apr 4, 2019
Merged

Task #3711 - jre of the properties file does not work under windows #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
060a2af
Task #3711 - jre of the properties file does not work under windows
lherschi Mar 22, 2019
d1c5623
jre of the properties file does not work under windows
lherschi Mar 22, 2019
b0cb8c9
Merge origin/Task_#3711 into Task_#3711
lherschi Apr 3, 2019
bb5da3b
Merge remote-tracking branch 'upstream/master' into Task_#3711
lherschi Apr 3, 2019
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
5 changes: 4 additions & 1 deletion netx/net/sourceforge/jnlp/controlpanel/JVMPanel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import javax.swing.event.DocumentListener;
import javax.swing.text.Document;
import net.sourceforge.jnlp.config.DeploymentConfiguration;
import net.sourceforge.jnlp.runtime.JNLPRuntime;
import net.sourceforge.jnlp.runtime.Translator;
import net.sourceforge.jnlp.util.logging.OutputController;
import net.sourceforge.jnlp.util.StreamUtils;
Expand Down Expand Up @@ -234,7 +235,9 @@ public static JvmValidationResult validateJvm(String cmd) {
validationResult += "<span color=\"red\">" + Translator.R("CPJVMnotDir") + "</span><br />";
latestOne = JvmValidationResult.STATE.NOT_DIR;
}
File javaFile = new File(cmd + File.separator + "bin" + File.separator + "java");
File javaFile = new File(cmd + File.separator + "bin" +
File.separator + "java" +
(JNLPRuntime.isWindows() ? ".exe" : ""));
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this more spread? If so, maybe it can get its own function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry i don't understand wath you mean with "Isn't this more spread?" I am not an English native speaker.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nor am I. I suspect that java part could forget .exe on more places. So maybe it would be good to ahve proper metho to appned .exe if necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All others I know are in the rust sources.

if (javaFile.isFile()) {
validationResult += "<span color=\"green\">" + Translator.R("CPJVMjava") + "</span><br />";
} else {
Expand Down
2 changes: 1 addition & 1 deletion rust-launcher/src/property.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ fn check_file_for_property(file: File, key: &str) -> Option<String> {
None => {}
Some(kvv) => {
if kvv.key.eq(key) {
return Some(escape_unicode(kvv.value));
return Some(escape_unicode(kvv.value.replace(r"\:", ":")));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please explain why it is necessary?
Will it not be soved by rust-lang/rust#27791 ? Which should be heading to stable soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The colon is a delimiter for Java properties. Therefore it is masked during serialization. When used in Java getProperty, it will automatically unmask. In Rust you have to take care of the unmasking yourself. How it should help escape_unicode (), does not open to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Colon xor equals is delimiter, depends what comes first. Isnt that?

Copy link
Contributor

Choose a reason for hiding this comment

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

ping?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The following line has itweb-settings written in my deployments.properties.

deployment.jre.dir=C\:\\Program Files\\AdoptOpenJDK\\jdk-8.0.202.08\\jre

Here is the equal sign the delimiter, but java has masked the colon anyway. Without the replace, this is an invalid path, which will simply not be accepted.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. gave sense now.

}
}
}
Expand Down
40 changes: 24 additions & 16 deletions rust-launcher/src/property_from_file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use property;
use hardcoded_paths;
use dirs_paths_helper as dh;
use os_access;

use std;
use std::string::String;
Expand All @@ -17,16 +18,16 @@ pub static KEY_ENABLE_LOGGING_TOSYSTEMLOG: &'static str = "deployment.log.syste


pub trait Validator {
fn validate(&self, s: &str) -> bool;
fn validate(&self, s: &str, os: &os_access::Os) -> bool;
fn get_fail_message(&self, key: &str, value: &str, file: &Option<std::path::PathBuf>) -> String;
}

pub struct JreValidator {}


impl Validator for JreValidator {
fn validate(&self, s: &str) -> bool {
verify_jdk_string(&s)
fn validate(&self, s: &str, os: &os_access::Os) -> bool {
verify_jdk_string(&s, os)
}

fn get_fail_message(&self, key: &str, value: &str, file: &Option<std::path::PathBuf>) -> String {
Expand All @@ -41,7 +42,7 @@ pub struct BoolValidator {}


impl Validator for BoolValidator {
fn validate(&self, s: &str) -> bool {
fn validate(&self, s: &str, _os: &os_access::Os) -> bool {
verify_bool_string(&s.to_string())
}

Expand All @@ -56,7 +57,7 @@ pub struct NotMandatoryPathValidator {}


impl Validator for NotMandatoryPathValidator {
fn validate(&self, _s: &str) -> bool {
fn validate(&self, _s: &str, _os: &os_access::Os) -> bool {
true
}

Expand Down Expand Up @@ -114,17 +115,22 @@ fn check_file_for_property(file: File, key: &str) -> Option<String> {
}


fn verify_jdk_string(spath: &str) -> bool {
fn verify_jdk_string(spath: &str, os: &os_access::Os) -> bool {
let mut file = std::path::PathBuf::from(spath);
file.push("bin");
file.push("java");
if !file.exists() {
false
} else if !dh::is_file(&file) {
false
} else {
true
for suffix in os.get_exec_suffixes() {
let mut bin_name = String::new();
write!(&mut bin_name, "java{}", suffix).expect("unwrap failed");
let full_path = file.join(bin_name);
if !full_path.exists() {
continue;
} else if !dh::is_file(&full_path) {
continue;
} else {
return true
}
}
false
}

/*tests*/
Expand All @@ -133,7 +139,7 @@ mod tests {
use std;
use std::fs::File;
use utils::tests_utils as tu;

fn get_jre_from_file(file: Option<std::path::PathBuf>) -> Option<String> {
super::get_property_from_file(file, super::JRE_PROPERTY_NAME)
}
Expand Down Expand Up @@ -243,15 +249,17 @@ mod tests {
#[test]
fn verify_jdk_string_verify_jdk_path_jdk_ok() {
let master_dir = tu::fake_jre(true);
let vs = super::verify_jdk_string(&master_dir.display().to_string());
let os = tu::TestLogger::create_new();
let vs = super::verify_jdk_string(&master_dir.display().to_string(), &os);
tu::debuggable_remove_dir(&master_dir);
assert_eq!(true, vs);
}

#[test]
fn verify_jdk_string_verify_jdk_path_jdk_bad() {
let master_dir = tu::fake_jre(false);
let vs = super::verify_jdk_string(&master_dir.display().to_string());
let os = tu::TestLogger::create_new();
let vs = super::verify_jdk_string(&master_dir.display().to_string(), &os);
tu::debuggable_remove_dir(&master_dir);
assert_eq!(false, vs);
}
Expand Down
2 changes: 1 addition & 1 deletion rust-launcher/src/property_from_files_resolver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ fn try_key_from_properties_files(logger: &os_access::Os, array: &[Option<std::pa
let mut info2 = String::new();
write!(&mut info2, "itw-rust-debug: located {} in file {}", value, file.clone().expect("file should be already verified").display()).expect("unwrap failed");
logger.log(&info2);
if validator.validate(&value) {
if validator.validate(&value, logger) {
return Some(value);
} else {
//the only output out of verbose mode
Expand Down