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

Loading template group from STGroupDir in app running on Tomcat 8 fails #293

Closed
Mooninaut opened this issue Feb 4, 2022 · 12 comments
Closed
Labels
Milestone

Comments

@Mooninaut
Copy link

Mooninaut commented Feb 4, 2022

StringTemplate version: 4.3.1

I'm migrating a web application from Tomcat 7 to Tomcat 8. It appears something in the class loader behavior has changed between the versions.

My app calls STGroupDir group = new STGroupDir("com/example/app/path/to/templates");

When running on Tomcat 7, after the line root = cl.getResource(dirName); at STGroupDir.java:72 the variable root has the value

jar:file:/path/to/exploded/app/WEB-INF/lib/my-component.jar!/com/example/app/path/to/templates

Running on Tomcat 8, root has the value

jar:file:/path/to/exploded/app/WEB-INF/lib/my-component.jar!/com/example/app/path/to/templates/

My app then calls ST st = group.getInstanceOf("/TemplateGroup/someTemplate"); to load someTemplate from ...path/to/templates/TemplateGroup.stg.

When load() calculates groupFileURL on line STGroupDir.java:128, the value is

jar:file:/path/to/exploded/app/WEB-INF/lib/my-component.jar!/com/example/app/path/to/templates//TemplateName.stg

The double slash prevents it from successfully loading the template group file, and getInstanceOf() returns null.

@parrt
Copy link
Member

parrt commented Feb 5, 2022

oh wow. Sorry about that. I'm probably not joining paths correctly. I'm currently focused on 1 billion other things but I would welcome a PR if you have time.

Mooninaut pushed a commit to Mooninaut/stringtemplate4 that referenced this issue Feb 7, 2022
Fix bug that prevented loading template groups from a template group dir if the path returned by getResource() ended in a '/' by removing the initial '/' from the parent if the root ends with '/'.
Mooninaut pushed a commit to Mooninaut/stringtemplate4 that referenced this issue Feb 23, 2022
Mooninaut pushed a commit to Mooninaut/stringtemplate4 that referenced this issue Feb 24, 2022
Fix bug that prevented loading template groups from a template group dir if the path returned by getResource() ended in a '/' by removing the initial '/' from the parent if the root ends with '/'.
Mooninaut pushed a commit to Mooninaut/stringtemplate4 that referenced this issue Feb 24, 2022
Mooninaut pushed a commit to Mooninaut/stringtemplate4 that referenced this issue Feb 24, 2022
@Mooninaut
Copy link
Author

@parrt I've submitted a PR to fix this and #276. Please let me know what you think!

@parrt
Copy link
Member

parrt commented Feb 27, 2022

I see that root = cl.getResource(dirName); at line 74 not 72. I wonder if we are looking at different versions of the software something. sorry for the hassles. I'm just trying to understand exactly what's going on and I haven't looked at the software in years. haha

@parrt
Copy link
Member

parrt commented Feb 27, 2022

line 128 in STGroupDir is the issue:

groupFileURL = new URI(root+parent+GROUP_FILE_EXTENSION).normalize().toURL();

which looks to be what you are fixing in your PR. I wonder if we can just use a proper URL or file path "join" to avoid the extra code with the special tests that you've created.

@Mooninaut
Copy link
Author

Mooninaut commented Mar 3, 2022

There is no simple URL/URI path join API in Java 1.6. The first line of STGroup.lookupTemplate() adds a leading / to the path, so root.resolve(parent + GROUP_FILE_EXTENSION) will treat parent as an absolute path, and strip the path from root. Now, STGroupDir.load() could remove that leading / and call resolve(), but at that point why bother? The URL/URI resolving code is very, very complex.

I've updated the PR to be more general, moving the logic to Misc.joinPathSegments() and testing that.

@parrt
Copy link
Member

parrt commented Mar 4, 2022

I would imagine we can move to Java 7 or 8 at this point. Is there something we can rely on in the new versions of Java?

@Mooninaut
Copy link
Author

I tried a Java 7 implementation using NIO at https://github.com/Mooninaut/stringtemplate4/tree/java1.7 , but it causes several dozen tests to fail on Windows. I don't know that it's worth it.

@parrt
Copy link
Member

parrt commented Mar 11, 2022

I didn't see any changes in that fork of yours but have you seen this that was linked to from the documentation?

https://maven.apache.org/plugin-developers/common-bugs.html#Converting_between_URLs_and_Filesystem_Paths

I'm going to look in there as well

@parrt
Copy link
Member

parrt commented Mar 11, 2022

What if we simply removed '//' from any URL string? @Mooninaut

@parrt
Copy link
Member

parrt commented Mar 11, 2022

I just pushed a new snapshot of 4.3.2 via maven. can you give it a try?

@parrt parrt modified the milestones: 4.1, 4.3.2 Apr 2, 2022
@Mooninaut
Copy link
Author

Works great!

@parrt
Copy link
Member

parrt commented May 27, 2022

hooray!

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

Successfully merging a pull request may close this issue.

2 participants