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

Incomplete implementation, leads to many downstream bugs #11

Open
StephanPreibisch opened this issue Oct 13, 2024 · 4 comments
Open

Incomplete implementation, leads to many downstream bugs #11

StephanPreibisch opened this issue Oct 13, 2024 · 4 comments

Comments

@StephanPreibisch
Copy link
Member

StephanPreibisch commented Oct 13, 2024

Hi @tpietzsch,

this line fails:

basePath = basePathURI == null ? null : new File( basePathURI );

if the URI file path does not start with "file:/", which is the normal case in the legacy code, the path will be e.g. "/home/data.xml" and not "file:/home/data.xml" ... My solution is this:

https://github.com/PreibischLab/multiview-reconstruction/blob/b102f2e3092f620a6e23f80061b547d362200548/src/main/java/util/URITools.java#L497-L506

Let's chat how to move forward, for now I implement it like that in my XMLIO's ... PreibischLab/multiview-reconstruction@38f1645

Alternatively, we could also always add "file:/" if it is missing ...

@tpietzsch
Copy link
Member

From your explanation, I don't understand what the problem is? What is "the URI file path"? basePathURI? and if that does not start with "file:/" it fails in what way?

@tpietzsch
Copy link
Member

Ok, I think I understand now.
The problem is actually in

public static URI loadPathURI( final Element parent, final String name, final String defaultRelativePath, final URI basePath )
{
final Element elem = parent.getChild( name );
final String path = ( elem == null ) ? defaultRelativePath : elem.getText();
final boolean isRelative = ( elem == null ) || elem.getAttributeValue( "type" ).equals( "relative" );
if ( isRelative )
{
if ( basePath == null )
return null;
else
return basePath.resolve( path );
}
else
{
try
{
return new URI( path );
}
catch ( URISyntaxException e )
{
return null;
}
}
}

in particular

This should add the "file:/" prefix for absolute paths (if no prefix is given). Otherwise, it will construct a URI (but a relative URI, without a scheme). And that's wrong.
In the location that you pointed out, everything is ok like it is. We can expect that the basePathURI is absolute, with scheme and all.

@StephanPreibisch
Copy link
Member Author

we also need to consider older XML's that do not start with a scheme ...

@StephanPreibisch
Copy link
Member Author

we'll figure the best way out tomorrow ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants