[core] Templating: limit compilation to supported extensions and constrain target paths #6598

jimschubert · 2020-06-09T12:00:45Z

I refactored template management in #6357. @wing328 found that an issue I had with one of the rust files was because the file was being processed by Mustache where it hadn't been before. This PR updates to process only files with an ending supported by the targeted templating engine.

This also fixes some Sonar recommendations, one of which may be a breaking change; template and output directories now may not include .. for directory traversals.

cc @OpenAPITools/generator-core-team

PR checklist

Read the contribution guidelines.
If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
Run the shell script ./bin/generate-samples.shto update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/config/java*.
File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

This forces the template adapters to only consider files with supported extensions, otherwise a direct file copy will occur. This also includes recommendation from Sonar analysis to ensure file writes are constrained to the target directory.

This enables core to only pass files through the templating engine if that file extension is supported by the engine. Otherwise, we do a direct file copy. This differs from logic in pre-5.0 in which everything but supporting files would be passed through the engine, and supporting files had different logic. This also fixes a bug(?) in pre-5.0 in which supporting files may not have honored resolution rules: template dir libraries, template dir root, embedded libraries, embedded root, common dir.

* master: [ci][cli] Moving ensures script to config-based batch generation of samples (#6509) Add Instana to the list of users (#6584) [rust][reqwest] fix broken export (#6586)

jimschubert · 2020-06-09T12:09:59Z

...enapi-generator-core/src/main/java/org/openapitools/codegen/api/TemplatingEngineAdapter.java

    default boolean templateExists(TemplatingExecutor generator, String templateFile) {
        return Arrays.stream(getFileExtensions()).anyMatch(ext -> {
-            int idx = templateFile.lastIndexOf(".");
+            int idx = templateFile.lastIndexOf('.');


lastIndexOf taking character is more performant

jimschubert · 2020-06-09T12:11:14Z

modules/openapi-generator/src/main/java/org/openapitools/codegen/TemplateManager.java

+            InputStream is;
+            try {
+                // look up the file using the same template resolution logic the adapters would use.
+                String fullTemplatePath = getFullTemplateFile(template);


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.

jimschubert · 2020-06-09T12:11:54Z

modules/openapi-generator/src/main/java/org/openapitools/codegen/TemplatingEngineLoader.java

@@ -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);


%n uses system-specific newline rather than newline only.

jimschubert · 2020-06-09T12:13:20Z

modules/openapi-generator/src/main/resources/rust/request.rs

@@ -113,7 +113,7 @@ impl Request {
        let mut path = self.path;
        for (k, v) in self.path_params {
            // replace {id} with the value of the id path param
-            {{=<% %>=}}path = path.replace(&format!("{{{}}}", k), &v);<%={{ }}=%>


This was added in the template management refactor because the file was being processed by Mustache engine (note file is request.rs not request.mustache and I missed this during the refactor)

* master: (142 commits) update python samples clarify direction of py client side validation flag (#6850) fix erronous cmd arg example for docker in readme (#6846) [BUG] [JAVA] Fix multiple files upload (#4803) (#6808) [kotlin][client] fix retrofit dependencies (#6836) [PowerShell] add more fields to be customized (#6835) [Java][WebClient]remove the dead code from java ApiClient.mustache (#6556) [PHP] Better handling of invalid data (array) (#6760) Make ApiClient in retrofit2 be able to use own OkHttpClient (#6699) mark python2 support in flask as deprecated (#6653) update samples [Java][jersey2] Add a getter for the User-Agent header value (#6831) Provides a default nil value for optional init parameters (#6827) [Java] Deprecate feignVersion option (#6824) [R] Enum R6Class Support, closes #3367 (#5728) [Rust][Client] Unify sync/async client structure (#6753) [php-ze-ph] Set required PHP version to ^7.2 (#6763) [Java][client][native][Gradle] Add missing jackson-databind-nullable (#6802) Improve sttpOpenApiClient generator (#6684) Update docker-tag-latest-release.yml ...

* master: Remove jimschubert from C# technical committee (#6865) Add Mock Server client modification feature (#6747) Update documentation with global propperty (#6863) [haskell-http-client] avoid depending on ParseTime,FormatTime (#6861) Update docker-tag-latest-release.yml

* master: (129 commits) [typescript-axios] add promise to bearer and oauth tokens (#7132) update doc [REQ] Added enumClassPrefix option to Go server generation (#7008) [Java][jersey2] Add helper methods for oneOf Java classes (#7115) [Kotlin][Retrofit2] fix missing import for file (#7121) adding handling for epoch dates in javascript ApiClient mustache file (#6497) (#6504) update doc comment out cpanm in travis [Kotlin] Rxjava3 support (#6998) [BUG][JAVA] Fix error handling in okhttp-gson async api client (#7089) Update to reset httpRepsonse.Body (#6948) [php-lumen] Set required PHP version to ^7.2.5 (#6949) [contrib][docs] Assert importance of title/description/repro steps (#7103) ISSUE-4222: Prevent conflicts with accept(s) local variables in generated Java RestTemplate ApiClient (#7101) [bug][core] Copy all attributes (not properties) on composed schemas when flattening models (#7106) [core] Add type and format properties to model of inline response (#6153) [PowerShell] better publishing workflow (#7114) [aspnetcore] Typo issues in docs and generated code (#7094) fix http signaure auth in build.sbt (#7110) fix for the issue facing spec invlolving arrayschema structure with ref (#6310) ...

* master: (27 commits) [WIP][python-exp] Force camelization of imports (#7186) Fixes #6942: Added ability to prepend a basePath to typescript-redux-query generators (#6943) [Typescript] Import path is invalid in windows. (#7175) Fix JaxRS Spec generator additional model types (#7180) [python{,-experimental}] Obey floating point timeouts provided to RESTClientObject.request(...) (#7154) [C#] Switch the spec to OAS v3 from v2 (#7176) [Javascript] Fixing the handling of non primitive types in paramToString (#7171) (#7172) [typescript-node] Fix invalid type when using node@10 and ES5 (#7133) Minor fix to github workflow badge [gradle] Enabling up-to-date checks and gradle caching for openapigenerator tasks (#6716) feat(csharp-netcore): Adding response headers to the ApiException (#7169) [ci] Verify supported JDK versions on master push (#7085) Issue #6830: Java server - Add getter to ApiException templates (#7150) update kotlin samples [Kotlin] Make ApiClient in jvm-retrofit2 be able to use own OkHttpClient (#6999) Sttp - wrap query params (#6884) Add a link to https://medium.com/@everisBrasil blog post (#7160) [C#][netcore] fix regular expression when it contains double quotes (#7147) remove duplicated cancellationToken in comment (#7148) update samples ...

jimschubert · 2020-08-12T02:41:07Z

Notice that php-lumen changes will be considered a breaking change in 5.0.0. This is the only generator which had mustache templates without mustache file extension.

cc @jebentier @dkarlovi @mandrean @jfastnacht @ackintosh @ybelenko @renepardon

ybelenko · 2020-08-12T16:23:26Z

@jimschubert Do they still need .mustache extension if they are source files from Lumen framework which shouldn't be modified?

jimschubert · 2020-08-12T22:16:04Z

@ybelenko no, anything without mustache extension will be copied as-is. Everything in my change will be processed through the mustache engine. Most of these files seemed to only have a mustache include for a header partial.

ybelenko · 2020-08-12T22:24:42Z

@jimschubert So, if I want to copy these files "as is" I should add header part inside and don't use mustache syntax, right?

ybelenko

I don't know why 5e01c65 commit contains samples/openapi3/client/petstore/java/jersey2-java8-special-characters/gradle/wrapper/gradle-wrapper.jar file, beside that everything related to PHP Lumen looks good to me.

* master: (89 commits) add structPrefix support to go-experimental (#7327) Add a link to SmartHR Tech Blog (#7324) Revert "Correct allOf with only one child schema (no discriminator)" (#7323) Correct allOf with only one child schema (no discriminator) (#6901) [Go]: Interface definitions for api functions (#5914) Update bug_report.md (#7320) update samples [Java][Client] Use java8 OffsetDateTime for clients (#7190) [java] Intro openApiNullable property to enable/disable OpenAPI Jackson Nullable library (#6154) [Spring Boot] update dependencies, mark java8 option as deprecated (#7306) Remove dot in golang type (#7307) [doc] Document usage of post-process file feature (#7315) fix http bear auth documentation for go clinets (#7312) [Extensions][Go][Java] Test x-auth-id-alias (#6642) [php-slim4] Move config to a separate file (#6971) [C][Client][Clang Static Analyzer] Remove the useless free operation for (#7309) Fix typescript-node generation when only models are generated (#7127) update spring config to use java8 (#7308) [C][Client][Clang Static Analyzer] Fix uninitialized argument value (#7305) [Java] remove deprecated jackson classes (#7304) ...

…ir contains period

jimschubert added 6 commits June 8, 2020 16:21

🎨 some Sonar quality fixes

71176ec

Remove relative directory reference in php-symfony-petstore.sh

e021c2b

[samples] Regenerate

17bfeb0

Merge branch 'master' into path-handling

b095f14

* master: [ci][cli] Moving ensures script to config-based batch generation of samples (#6509) Add Instana to the list of users (#6584) [rust][reqwest] fix broken export (#6586)

jimschubert added the Enhancement: General label Jun 9, 2020

jimschubert added this to the 5.0.0 milestone Jun 9, 2020

[samples] Regenerate

bbfc66d

jimschubert commented Jun 9, 2020

View reviewed changes

jimschubert added 6 commits July 3, 2020 14:49

Regenerate samples

36e8a3b

[php-lumen] Rename mustache templates with correct file extension

5e01c65

ybelenko approved these changes Aug 19, 2020

View reviewed changes

jimschubert added 4 commits September 1, 2020 21:11

[samples] Regenerated

73d58ac

Add target/outDir to error requiring target under outDir

837ff02

[bug][python-blueplanet] Fix docs writing to invalid output when outD…

37fce4e

…ir contains period

jimschubert merged commit a6d30ca into master Sep 2, 2020

jimschubert deleted the path-handling branch September 2, 2020 19:52

zippolyte mentioned this pull request Sep 10, 2020

[core] Use canonical path for template dir #7388

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Templating: limit compilation to supported extensions and constrain target paths #6598

[core] Templating: limit compilation to supported extensions and constrain target paths #6598

jimschubert commented Jun 9, 2020

jimschubert Jun 9, 2020

jimschubert Jun 9, 2020

jimschubert Jun 9, 2020

jimschubert Jun 9, 2020

jimschubert commented Aug 12, 2020

ybelenko commented Aug 12, 2020

jimschubert commented Aug 12, 2020

ybelenko commented Aug 12, 2020

ybelenko left a comment

[core] Templating: limit compilation to supported extensions and constrain target paths #6598

[core] Templating: limit compilation to supported extensions and constrain target paths #6598

Conversation

jimschubert commented Jun 9, 2020

PR checklist

jimschubert Jun 9, 2020

Choose a reason for hiding this comment

jimschubert Jun 9, 2020

Choose a reason for hiding this comment

jimschubert Jun 9, 2020

Choose a reason for hiding this comment

jimschubert Jun 9, 2020

Choose a reason for hiding this comment

jimschubert commented Aug 12, 2020

ybelenko commented Aug 12, 2020

jimschubert commented Aug 12, 2020

ybelenko commented Aug 12, 2020

ybelenko left a comment

Choose a reason for hiding this comment