Fix bug introduced by #308 when converting URLs to local paths on Windows

[jordanpadams]

🗒️ Summary

When using Path.of() we need to pass a URI on a Windows machine, not the Path.

Resolves #809

⚙️ Test Data and/or Report

See regression tests to verify this doesn't break other tests.

♻️ Related Issues

Resolves #809

[jordanpadams]

@al-niessner can you poke at this when you can? This is breaking validate on Windows.

[al-niessner]

@al-niessner can you poke at this when you can? This is breaking validate on Windows.

@jordanpadams

Are you saying that getURI() is not the solution over getPath() so it still does not work? If so, can we undo these changes. I dislike changes that do not fix anything but add complication (new exception handling for no reason).

[al-niessner]

@jordanpadams

Just read the ticket and error. The problem is not getPath(). It is the URL is malformed somehow according the windoze experts. Also wikipedia

Let me see if I can round up a windoze box and test a little bit.

[al-niessner]

@jordanpadams

Nope. We are both wrong it seems. JDK 17? I have eclipse running jdk 17 running a simple test and they both work. You can run the test on your windows to see how it does:

Code:

$ cat ~/Scratch/UrlOnWIn.java 
package test;

import java.net.MalformedURLException;
import java.net.URL;

public class UrlOnWIn {
	public static void main(String[] args) {
		try {
			URL a = new URL("file:/a:/foo/bar");
			URL b = new URL("file:///a:/foo/bar");
			
			try { a.getPath(); System.out.println ("a worked"); }
			catch (Exception e) { System.out.println ("a failed"); }
			try { b.getPath(); System.out.println ("b worked"); }
			catch (Exception e) { System.out.println ("b failed"); }
		} catch (MalformedURLException e) {
			e.printStackTrace();
		}
	}
}

Result:

a worked
b worked

[al-niessner]

@jordanpadams

LOL - experts were right. The failure is from the Path.of() not in the URL.getPath(). Gads. Anyway, update the test and we are clearly forming the URL incorrectly when we make on windows.

Code:

package test;

import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.Path;

public class UrlOnWIn {
	public static void main(String[] args) {
		try {
			URL a = new URL("file:/a:/foo/bar");
			URL b = new URL("file:///a:/foo/bar");
			
			try { a.getPath(); System.out.println ("a worked"); System.out.println (Path.of(a.getPath()));}
			catch (Exception e) { System.out.println ("a failed"); }
			try { b.getPath(); System.out.println ("b worked"); }
			catch (Exception e) { System.out.println ("b failed"); System.out.println (Path.of(a.getPath()));}
		} catch (MalformedURLException e) {
			e.printStackTrace();
		}
	}
}

Result:

a worked
a failed
b worked

So, I can chase this back if you like but other than doing simple tests in wine, I cannot really do much.

[al-niessner]

@jordanpadams

Flipping through browser tabs and noticed test is not right. Give me a moment. Have to start wine again.

[al-niessner]

@jordanpadams

Okay, test is now more pedantic. It is a mess because it is so hard for me to use windoze. The answer is, unfortunately, that we need the URI and the URL needs to be fixed. The test results show that if you need/want the x: to become \x in Path, then you need the right number of flipping slashes in the URL. I expanded the testing but it is still hard to read the output but the hint is bb is right and all others are wrong. Back when we create the URL probably should set the protocol rather than prefix with what seems to work.

Code:

package test;

import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Path;

public class UrlOnWIn {
	public static void main(String[] args) {
		try {
			URL a = new URL("file:/a:/foo/bar");
			URL b = new URL("file://b:/foo/bar");
			URL c = new URL("file://c:/foo/bar");
			
			try { System.out.println ("URL: " + a.getPath()); System.out.println ("Path: " + Path.of(a.getPath())); }
			catch (Exception e) { System.out.println ("a failed"); }
			try { System.out.println ("URL: " + b.getPath()); System.out.println ("Path: " + Path.of(b.getPath())); }
			catch (Exception e) { System.out.println ("b failed"); }
			try { System.out.println ("URL: " + c.getPath()); System.out.println ("Path: " + Path.of(c.getPath())); }
			catch (Exception e) { System.out.println ("c failed"); }
			
			try { System.out.println ("URL: " + a.getPath()); System.out.println ("Path: " + Path.of(a.toURI())); }
			catch (Exception e) { System.out.println ("a failed"); }
			try { System.out.println ("URL: " + b.getPath()); System.out.println ("Path: " + Path.of(b.toURI())); }
			catch (Exception e) { System.out.println ("b failed"); }
			try { System.out.println ("URL: " + c.getPath()); System.out.println ("Path: " + Path.of(c.toURI())); }
			catch (Exception e) { System.out.println ("c failed"); }
			
			URI aa = new URI("file:/aa:/foo/bar");
			URI bb = new URI("file://bb:/foo/bar");
			URI cc = new URI("file:///cc:/foo/bar");
			try { System.out.println ("URI: " + aa.getPath()); System.out.println ("Path: " + Path.of(aa)); }
			catch (Exception e) { System.out.println ("aa failed"); }
			try { System.out.println ("URI: " + bb.getPath()); System.out.println ("Path: " + Path.of(bb)); }
			catch (Exception e) { System.out.println ("bb failed"); }
			try { System.out.println ("URI: " + cc.getPath()); System.out.println ("Path: " + Path.of(cc)); }
			catch (Exception e) { System.out.println ("cc failed"); }			
		} catch (MalformedURLException e) {
			e.printStackTrace();
		} catch (URISyntaxException e1) {
			e1.printStackTrace();
		}
	}
}

Output:

URL: /a:/foo/bar
a failed
URL: /foo/bar
Path: \foo\bar
URL: /foo/bar
Path: \foo\bar
URL: /a:/foo/bar
Path: a:\foo\bar
URL: /foo/bar
Path: \\b\foo\bar
URL: /foo/bar
Path: \\c\foo\bar
URI: /aa:/foo/bar
aa failed
URI: /foo/bar
Path: \\bb\foo\bar
URI: /cc:/foo/bar
cc failed

[jordanpadams]

@al-niessner just did a quick test of this and it looks like this is a weird thing with the URL vs. URI toStrings?

[main] DEBUG gov.nasa.pds.validate.ValidateLauncher - Path.of(t).toUri(): file:///absolute/path/to/ff_char.xml
[main] DEBUG gov.nasa.pds.validate.ValidateLauncher - Path.of(t).toUri().toURL():  file:/absolute/path/to/ff_char.xml

So I'm not sure what to do here? Do we need to fix the URL? Per this, it sounds like it is a difference in the toString()? https://stackoverflow.com/questions/6098472/pass-a-local-file-in-to-url-in-java

[jordanpadams]

@al-niessner nevermind, I think I see the issue. Once convert to URL, it loses the appropriate path information.

        url = Path.of(t).toUri().toURL();
        LOG.debug("URI: {}", url.toURI());
        LOG.debug("URL: {}", url);

[main] DEBUG gov.nasa.pds.validate.ValidateLauncher - URI: file:/path/github681/ff_char.xml
[main] DEBUG gov.nasa.pds.validate.ValidateLauncher - URL: file:/path/github681/ff_char.xml

[jordanpadams]

@al-niessner I have been testing this like crazy and cannot figure out a workaround using URL objects. I think we will need to completely refactor out URLs to URIs throughout the code.

That being said, this code appears to now run successfully on windows. Are you saying this does not/should not work? Or that this is bound to break elsewhere in the code if we don't fix this path issue?

[jordanpadams]

Merging this PR for now since it does fix something. But per comment above, may not fix everything.

[msbentley]

Hi @jordanpadams when will a version with this fix be released?