-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[core] Templating: limit compilation to supported extensions and constrain target paths #6598
Changes from all commits
d3b28d2
71176ec
ea9db33
e021c2b
17bfeb0
b095f14
bbfc66d
3574e9c
36e8a3b
48ca3aa
f3db22c
4c9b64f
5e01c65
164fd7d
73d58ac
837ff02
37fce4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
package org.openapitools.codegen; | ||
|
||
import org.apache.commons.io.IOUtils; | ||
import org.apache.commons.lang3.StringUtils; | ||
import org.openapitools.codegen.api.TemplatePathLocator; | ||
import org.openapitools.codegen.api.TemplateProcessor; | ||
|
@@ -12,14 +13,8 @@ | |
|
||
import java.io.*; | ||
import java.nio.charset.StandardCharsets; | ||
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.nio.file.Paths; | ||
import java.nio.file.StandardCopyOption; | ||
import java.util.Arrays; | ||
import java.util.Map; | ||
import java.util.Objects; | ||
import java.util.Scanner; | ||
import java.nio.file.*; | ||
import java.util.*; | ||
import java.util.regex.Pattern; | ||
|
||
/** | ||
|
@@ -59,6 +54,10 @@ private String getFullTemplateFile(String name) { | |
throw new TemplateNotFoundException(name); | ||
} | ||
|
||
if (name == null || name.contains("..")) { | ||
throw new IllegalArgumentException("Template location must be constrained to template directory."); | ||
} | ||
|
||
return template; | ||
} | ||
|
||
|
@@ -104,7 +103,12 @@ public static String getCPResourcePath(final String name) { | |
* @param name The location of the template | ||
* @return The raw template contents | ||
*/ | ||
@SuppressWarnings({"java:S112"}) | ||
// ignored rule java:S112 as RuntimeException is used to match previous exception type | ||
public String readTemplate(String name) { | ||
if (name == null || name.contains("..")) { | ||
throw new IllegalArgumentException("Template location must be constrained to template directory."); | ||
} | ||
try { | ||
Reader reader = getTemplateReader(name); | ||
if (reader == null) { | ||
|
@@ -118,22 +122,31 @@ public String readTemplate(String name) { | |
throw new RuntimeException("can't load template " + name); | ||
} | ||
|
||
@SuppressWarnings("squid:S2095") | ||
// ignored rule as used in the CLI and it's required to return a reader | ||
@SuppressWarnings({"squid:S2095", "java:S112"}) | ||
// ignored rule squid:S2095 as used in the CLI and it's required to return a reader | ||
// ignored rule java:S112 as RuntimeException is used to match previous exception type | ||
public Reader getTemplateReader(String name) { | ||
InputStream is = null; | ||
try { | ||
is = this.getClass().getClassLoader().getResourceAsStream(getCPResourcePath(name)); | ||
if (is == null) { | ||
is = new FileInputStream(new File(name)); // May throw but never return a null value | ||
} | ||
InputStream is = getInputStream(name); | ||
return new InputStreamReader(is, StandardCharsets.UTF_8); | ||
} catch (FileNotFoundException e) { | ||
LOGGER.error(e.getMessage()); | ||
throw new RuntimeException("can't load template " + name); | ||
} | ||
} | ||
|
||
private InputStream getInputStream(String name) throws FileNotFoundException { | ||
InputStream is; | ||
is = this.getClass().getClassLoader().getResourceAsStream(getCPResourcePath(name)); | ||
if (is == null) { | ||
if (name == null || name.contains("..")) { | ||
throw new IllegalArgumentException("Template location must be constrained to template directory."); | ||
} | ||
is = new FileInputStream(new File(name)); // May throw but never return a null value | ||
} | ||
return is; | ||
} | ||
|
||
/** | ||
* Writes data to a compiled template | ||
* | ||
|
@@ -145,18 +158,32 @@ public Reader getTemplateReader(String name) { | |
*/ | ||
@Override | ||
public File write(Map<String, Object> data, String template, File target) throws IOException { | ||
String templateContent = this.engineAdapter.compileTemplate(this, data, template); | ||
return writeToFile(target.getPath(), templateContent); | ||
if (this.engineAdapter.handlesFile(template)) { | ||
// Only pass files with valid endings through template engine | ||
String templateContent = this.engineAdapter.compileTemplate(this, data, template); | ||
return writeToFile(target.getPath(), templateContent); | ||
} else { | ||
// Do a straight copy of the file if not listed as supported by the template engine. | ||
InputStream is; | ||
try { | ||
// look up the file using the same template resolution logic the adapters would use. | ||
String fullTemplatePath = getFullTemplateFile(template); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixes what I believe may have been a bug prior to this change… supporting files may not have resolved using the libraries path rules appropriately. |
||
is = getInputStream(fullTemplatePath); | ||
} catch (TemplateNotFoundException ex) { | ||
is = new FileInputStream(Paths.get(template).toFile()); | ||
} | ||
return writeToFile(target.getAbsolutePath(), IOUtils.toByteArray(is)); | ||
} | ||
} | ||
|
||
@Override | ||
public void ignore(Path path, String context) { | ||
LOGGER.info("Ignored {} ({})", path.toString(), context); | ||
LOGGER.info("Ignored {} ({})", path, context); | ||
} | ||
|
||
@Override | ||
public void skip(Path path, String context) { | ||
LOGGER.info("Skipped {} ({})", path.toString(), context); | ||
LOGGER.info("Skipped {} ({})", path, context); | ||
} | ||
|
||
/** | ||
|
@@ -189,23 +216,23 @@ public File writeToFile(String filename, byte[] contents) throws IOException { | |
try { | ||
tempFile = writeToFileRaw(tempFilename, contents); | ||
if (!filesEqual(tempFile, outputFile)) { | ||
LOGGER.info("writing file " + filename); | ||
LOGGER.info("writing file {}", filename); | ||
Files.move(tempFile.toPath(), outputFile.toPath(), StandardCopyOption.REPLACE_EXISTING); | ||
tempFile = null; | ||
} else { | ||
LOGGER.info("skipping unchanged file " + filename); | ||
LOGGER.info("skipping unchanged file {}", filename); | ||
} | ||
} finally { | ||
if (tempFile != null && tempFile.exists()) { | ||
try { | ||
tempFile.delete(); | ||
Files.delete(tempFile.toPath()); | ||
} catch (Exception ex) { | ||
LOGGER.error("Error removing temporary file " + tempFile, ex); | ||
LOGGER.error("Error removing temporary file {}", tempFile, ex); | ||
} | ||
} | ||
} | ||
} else { | ||
LOGGER.info("writing file " + filename); | ||
LOGGER.info("writing file {}", filename); | ||
outputFile = writeToFileRaw(filename, contents); | ||
} | ||
|
||
|
@@ -216,7 +243,7 @@ private File writeToFileRaw(String filename, byte[] contents) throws IOException | |
// Use Paths.get here to normalize path (for Windows file separator, space escaping on Linux/Mac, etc) | ||
File output = Paths.get(filename).toFile(); | ||
if (this.options.isSkipOverwrite() && output.exists()) { | ||
LOGGER.info("skip overwrite of file " + filename); | ||
LOGGER.info("skip overwrite of file {}", filename); | ||
return output; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,11 @@ | |
import java.util.ServiceLoader; | ||
|
||
public class TemplatingEngineLoader { | ||
private TemplatingEngineLoader() { | ||
throw new IllegalStateException("Utility class"); | ||
} | ||
|
||
@SuppressWarnings({"java:S112"}) // ignore java:S112 as generic RuntimeException is acceptable here | ||
public static TemplatingEngineAdapter byIdentifier(String id) { | ||
ServiceLoader<TemplatingEngineAdapter> loader = ServiceLoader.load(TemplatingEngineAdapter.class, TemplatingEngineLoader.class.getClassLoader()); | ||
|
||
|
@@ -37,7 +42,7 @@ public static TemplatingEngineAdapter byIdentifier(String id) { | |
// Attempt to load skipping SPI | ||
return (TemplatingEngineAdapter) Class.forName(id).getDeclaredConstructor().newInstance(); | ||
} catch (Exception e) { | ||
throw new RuntimeException(String.format(Locale.ROOT, "Couldn't load template engine adapter %s. Available options: \n%s", id, sb.toString()), e); | ||
throw new RuntimeException(String.format(Locale.ROOT, "Couldn't load template engine adapter %s. Available options: %n%s", id, sb.toString()), e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lastIndexOf taking character is more performant