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

Fixed template path resolvement to work also on Windows OS. #8284

Merged
merged 3 commits into from
Jun 8, 2018
Merged

Fixed template path resolvement to work also on Windows OS. #8284

merged 3 commits into from
Jun 8, 2018

Conversation

thomas-worm-datev
Copy link

No description provided.

}
TemplateLoader templateLoader = null;
if (config.additionalProperties().get(CodegenConstants.TEMPLATE_DIR) != null) {
templateLoader = new FileTemplateLoader(config.templateDir(), ".mustache");
templateLoader = new FileTemplateLoader(config.templateDir().replace("\\", "/"), ".mustache");
Copy link
Contributor

Choose a reason for hiding this comment

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

hey @thomas-worm-datev, i think this could work but it's not the causes of the issue, i tested locally and when we use a custom templates it crash.

The real issue is this statement:

templateFile = templateFile.replace("\\", "/");

at line 568

if you're agree, please assign permissions to HugoMario to your repo and i can help sending the fix from your PR.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @HugoMario,

I first tested with your suggested line, but that solved the problem only partially. I got error that V2\JavaSpring/api.mustache could not be found.

This was caused from the line
final Handlebars handlebars = new Handlebars(templateLoader);

So I found out that even the templateDir path has to be handled with a safe path handling.

Best Regards,
Thomas

Copy link
Contributor

@HugoMario HugoMario Jun 8, 2018

Choose a reason for hiding this comment

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

I'm not sure if that line is the issue, try using a custom templates in a windows OS with your changes and you'll see my point.

@HugoMario HugoMario merged commit 5752e5c into swagger-api:3.0.0 Jun 8, 2018
@HugoMario
Copy link
Contributor

thanks a lot @thomas-worm-datev !!!

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

Successfully merging this pull request may close these issues.

2 participants