Sanitize IDL text written into generated comments and strings

Build/Dockerfile

@@ -68,9 +68,11 @@ RUN dnf -y install \
 RUN echo "source /opt/rh/gcc-toolset-9/enable" >> /etc/bashrc
 
 # ---- Emscripten SDK (emsdk) ----
+# Emscripten 4.0.22 and later require Python 3.10, which is not available for CentOS 8.
+ARG EMSDK_VERSION=4.0.21
 ENV EMSDK=/opt/emsdk
-RUN git clone https://github.com/emscripten-core/emsdk.git ${EMSDK} && \
-    bash -lc "cd ${EMSDK} && ./emsdk install latest && ./emsdk activate latest"
+RUN git clone --branch ${EMSDK_VERSION} --depth 1 https://github.com/emscripten-core/emsdk.git ${EMSDK} && \
+    bash -lc "cd ${EMSDK} && ./emsdk install ${EMSDK_VERSION} && ./emsdk activate ${EMSDK_VERSION}"
 
 # Source emsdk env in interactive shells
 RUN echo "source ${EMSDK}/emsdk_env.sh" >> /etc/bashrc

Source/buildbindingjava.go

@@ -536,7 +536,7 @@ func writeJavaClassMethodImplementation(method ComponentDefinitionMethod, w Lang
 
 	initCallParameters = callFunctionParameters
 	w.Writeln("  /**")
-	w.Writeln("   * " + method.MethodDescription)
+	w.Writeln("   * %s", sanitizeCommentText(method.MethodDescription, "*/"))
 	w.Writeln("   *")
 
 	OutFieldCount := 0
@@ -563,10 +563,10 @@ func writeJavaClassMethodImplementation(method ComponentDefinitionMethod, w Lang
 		}
 		if (param.ParamPass == "out" || param.ParamPass == "return") {
 			if OutFieldCount == 1 {
-				w.Writeln("   * @return %s", param.ParamDescription)
+				w.Writeln("   * @return %s", sanitizeCommentText(param.ParamDescription, "*/"))
 			}		
 		} else {
-			w.Writeln("   * @param %s %s", MakeFirstLowerCase(param.ParamName), param.ParamDescription)
+			w.Writeln("   * @param %s %s", MakeFirstLowerCase(param.ParamName), sanitizeCommentText(param.ParamDescription, "*/"))
 		}
 
 		switch param.ParamPass {
@@ -835,7 +835,7 @@ func writeJavaClassMethodImplementation(method ComponentDefinitionMethod, w Lang
 		w.Writeln("  public static class %sResult {", method.MethodName)
 		for _,ReturnParam := range(ReturnTuple) {
 			w.Writeln("    /**")
-			w.Writeln("     * " + ReturnParam.ParamDescription)
+			w.Writeln("     * %s", sanitizeCommentText(ReturnParam.ParamDescription, "*/"))
 			w.Writeln("     */")
 			w.Writeln("    public %s %s;", ReturnParam.ParamType, ReturnParam.ParamName)
 			w.Writeln("")

Source/buildbindingpython.go

@@ -307,7 +307,7 @@ func buildDynamicPythonImplementation(componentdefinition ComponentDefinition, w
 				return ReservedKeywordExit(pythonBindingFile, "Function type definition uses a reserved keyword : %s", _func.FunctionName)
 			}
 			w.Writeln("'''Definition of %s", _func.FunctionName)
-			w.Writeln("    %s", _func.FunctionDescription)
+			w.Writeln("    %s", sanitizeCommentText(_func.FunctionDescription, "'''"))
 			w.Writeln("'''")
 			arguments := "ctypes.c_void_p"
 			for j := 0; j<len(_func.Params); j++ {

Source/componentdefinition.go

@@ -483,7 +483,7 @@ func errorDescriptionIsValid (name string) bool {
 	var IsValidIdentifier = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_+\\-:,.=!/#@%$* ]*$").MatchString
 
 	if (name != "") {
-		return IsValidIdentifier(name);
+		return IsValidIdentifier(name) && commentTextIsSafe(name);
 	}
 
 	return false;
@@ -507,6 +507,9 @@ func checkOptions(options[] ComponentDefinitionEnumOption) (error) {
 		if optionLowerNameList[strings.ToLower(option.Name)] {
 			return fmt.Errorf("duplicate option name \"%s\"", option.Name);
 		}
+		if !commentTextIsSafe(option.Description) {
+			return fmt.Errorf("invalid description for option \"%s\"", option.Name)
+		}
 		optionValueList[option.Value] = true
 		optionLowerNameList[strings.ToLower(option.Name)] = true
 	}
@@ -527,6 +530,9 @@ func (component *ComponentDefinition) checkEnums() (error) {
 		if (enumLowerNameList[strings.ToLower(enum.Name)]) {
 			return fmt.Errorf("duplicate enum name \"%s\"", enum.Name);
 		}
+		if !commentTextIsSafe(enum.Description) {
+			return fmt.Errorf("invalid description for enum \"%s\"", enum.Name)
+		}
 
 		err := checkOptions(enum.Options)
 		if err != nil {
@@ -657,6 +663,11 @@ func (component *ComponentDefinition) checkFunctionTypes() (error) {
 		if len(function.FunctionDescription) > 0 && !descriptionIsValid(function.FunctionDescription) {
 			return fmt.Errorf ("invalid function description \"%s\" in functiontype \"%s\"", function.FunctionDescription, function.FunctionName);
 		}
+		for _, param := range function.Params {
+			if len(param.ParamDescription) > 0 && !descriptionIsValid(param.ParamDescription) {
+				return fmt.Errorf("invalid description for parameter \"%s\" in functiontype \"%s\"", param.ParamName, function.FunctionName)
+			}
+		}
 
 		functionLowerNameList[strings.ToLower(function.FunctionName)] = true
 		(*functionNameList)[function.FunctionName] = true
@@ -748,7 +759,7 @@ func (component *ComponentDefinition) checkMethod(method ComponentDefinitionMeth
 		if !nameIsValidIdentifier(param.ParamName) {
 			return fmt.Errorf("invalid param name \"%s\" in method \"%s.%s\"", param.ParamName, className, method.MethodName);
 		}
-		if !descriptionIsValid(method.MethodDescription) {
+		if len(param.ParamDescription) > 0 && !descriptionIsValid(param.ParamDescription) {
 			return fmt.Errorf("invalid description for parameter \"%s.%s(... %s ...)\"", className, method.MethodName, param.ParamName);
 		}
 		if (paramNameList[strings.ToLower(param.ParamName)]) {
@@ -884,11 +895,16 @@ func nameIsValidIdentifier(name string) bool {
 func descriptionIsValid(description string) bool {
 	var IsValidMethodDescription = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_\\\\/+\\-:,.=!?()';&#@%$* |]*$").MatchString
 	if (description != "") {
-		return IsValidMethodDescription(description);
+		return IsValidMethodDescription(description) && commentTextIsSafe(description);
 	}
 	return false;
 }
 
+// commentTextIsSafe returns false if s could terminate a C-style or Pascal comment it is written into
+func commentTextIsSafe(s string) bool {
+	return sanitizeCommentText(s, "*/") == s && sanitizeCommentText(s, "*)") == s
+}
+
 func threadSafetyOptionIsValid(threadSafetyOption string) bool {
 	switch threadSafetyOption {
 	case "none", "strict", "soft":
@@ -1025,6 +1041,14 @@ func (component *ComponentDefinition) checkComponentHeader() (error) {
 	if !baseNameIsValid(component.BaseName) {
 		return errors.New ("Invalid BaseName");
 	}
+	if headerTextNeedsSanitizing(component.Copyright) {
+		log.Printf("Warning: the copyright of component \"%s\" contains characters that will be sanitized in generated license headers", component.NameSpace)
+	}
+	for i, line := range component.License.Lines {
+		if headerTextNeedsSanitizing(line.Value) {
+			log.Printf("Warning: license line %d of component \"%s\" contains characters that will be sanitized in generated license headers", i+1, component.NameSpace)
+		}
+	}
 	return nil
 }
 

Source/componentdefinition_test.go

@@ -0,0 +1,138 @@
+package main
+
+import (
+	"testing"
+)
+
+func TestCheckMethodValidatesParamDescriptions(t *testing.T) {
+	tests := []struct {
+		name        string
+		description string
+		valid       bool
+	}{
+		{"regular description", "The new value of this Variable", true},
+		{"empty description", "", true},
+		{"C comment breakout", "x */ int injected = 1; /* y", false},
+		{"Pascal comment breakout", "x *) begin end; (* y", false},
+	}
+
+	var component ComponentDefinition
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			method := ComponentDefinitionMethod{
+				MethodName:        "SetValue",
+				MethodDescription: "Sets the value",
+				Params: []ComponentDefinitionParam{
+					{ParamName: "Value", ParamType: "double", ParamPass: "in", ParamDescription: test.description},
+				},
+			}
+			err := component.checkMethod(method, "Variable")
+			if test.valid && err != nil {
+				t.Errorf("checkMethod rejected param description %q: %v", test.description, err)
+			}
+			if !test.valid && err == nil {
+				t.Errorf("checkMethod accepted param description %q", test.description)
+			}
+		})
+	}
+}
+
+func TestDescriptionsRejectCommentBreakout(t *testing.T) {
+	safe := []string{"Returns the value", "A pointer * to the buffer", "Path C:\\Data", "Size in 100% units"}
+	unsafe := []string{
+		"x */ int injected = 1; /* y",
+		"x /* y",
+		"x *) begin end; (* y",
+		"x \\u002a/ class Evil {} /\\u002a",
+		"x ends with a backslash \\",
+		"x ends with a trigraph ??/",
+	}
+	for _, s := range safe {
+		if !descriptionIsValid(s) {
+			t.Errorf("descriptionIsValid(%q) = false, expected true", s)
+		}
+		if !commentTextIsSafe(s) {
+			t.Errorf("commentTextIsSafe(%q) = false, expected true", s)
+		}
+	}
+	for _, s := range unsafe {
+		if descriptionIsValid(s) {
+			t.Errorf("descriptionIsValid(%q) = true, expected false", s)
+		}
+		if commentTextIsSafe(s) {
+			t.Errorf("commentTextIsSafe(%q) = true, expected false", s)
+		}
+	}
+	if errorDescriptionIsValid("x */ int injected = 1; /* y") {
+		t.Errorf("errorDescriptionIsValid accepted a comment breakout")
+	}
+	if !errorDescriptionIsValid("The value is out of range") {
+		t.Errorf("errorDescriptionIsValid rejected a regular description")
+	}
+}
+
+func TestCheckEnumsValidatesDescriptions(t *testing.T) {
+	tests := []struct {
+		name              string
+		enumDescription   string
+		optionDescription string
+		valid             bool
+	}{
+		{"regular descriptions", "The color", "Red color", true},
+		{"empty descriptions", "", "", true},
+		{"enum comment breakout", "x */ int injected = 1; /* y", "", false},
+		{"option comment breakout", "", "x */ int injected = 1; /* y", false},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var component ComponentDefinition
+			component.NameMapsLookup.enumMap = make(map[string]bool)
+			component.Enums = []ComponentDefinitionEnum{{
+				Name:        "Color",
+				Description: test.enumDescription,
+				Options:     []ComponentDefinitionEnumOption{{Name: "Red", Value: 0, Description: test.optionDescription}},
+			}}
+			err := component.checkEnums()
+			if test.valid && err != nil {
+				t.Errorf("checkEnums rejected valid descriptions: %v", err)
+			}
+			if !test.valid && err == nil {
+				t.Errorf("checkEnums accepted enum description %q and option description %q", test.enumDescription, test.optionDescription)
+			}
+		})
+	}
+}
+
+func TestCheckFunctionTypesValidatesParamDescriptions(t *testing.T) {
+	tests := []struct {
+		name        string
+		description string
+		valid       bool
+	}{
+		{"regular description", "The progress", true},
+		{"empty description", "", true},
+		{"C comment breakout", "x */ int injected = 1; /* y", false},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var component ComponentDefinition
+			component.NameMapsLookup.functionTypeMap = make(map[string]bool)
+			component.Functions = []ComponentDefinitionFunctionType{{
+				FunctionName:        "ProgressCallback",
+				FunctionDescription: "Callback to report progress",
+				Params: []ComponentDefinitionParam{
+					{ParamName: "Progress", ParamType: "single", ParamPass: "in", ParamDescription: test.description},
+				},
+			}}
+			err := component.checkFunctionTypes()
+			if test.valid && err != nil {
+				t.Errorf("checkFunctionTypes rejected param description %q: %v", test.description, err)
+			}
+			if !test.valid && err == nil {
+				t.Errorf("checkFunctionTypes accepted param description %q", test.description)
+			}
+		})
+	}
+}

Source/languagewriter.go

@@ -37,9 +37,14 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"regexp"
 	"strings"
+	"unicode"
 )
 
+// javaUnicodeEscape matches Java unicode escapes, which Java decodes everywhere in a source file, including comments
+var javaUnicodeEscape = regexp.MustCompile(`\\(u+[0-9a-fA-F]{4})`)
+
 // LanguageWriter is a wrapper around a io.Writer that handles indentation
 type LanguageWriter struct {
 	Indentation  int
@@ -151,6 +156,61 @@ func WriteLicenseHeader(w io.Writer, component ComponentDefinition, abstract str
 	writeLicenseHeaderEx(w, component, abstract, includeVersion, "/*", "*/")
 }
 
+// replaceUntilStable replaces old by new until old no longer occurs in s
+func replaceUntilStable(s string, old string, new string) string {
+	for strings.Contains(s, old) {
+		s = strings.ReplaceAll(s, old, new)
+	}
+	return s
+}
+
+// sanitizeCommentText neutralizes character sequences in s that could terminate a comment or string literal
+// which is closed by commentEnd, or otherwise alter the code that follows it. s is written as a single line.
+func sanitizeCommentText(s string, commentEnd string) string {
+	s = strings.Map(func(r rune) rune {
+		if unicode.IsControl(r) {
+			return ' '
+		}
+		return r
+	}, s)
+
+	switch commentEnd {
+	case "*/":
+		// An escape such as \u002a/ would otherwise close the comment in Java.
+		s = javaUnicodeEscape.ReplaceAllString(s, "\\ $1")
+		// A nested opening sequence triggers -Wcomment, which fails builds that use -Werror.
+		s = replaceUntilStable(s, "*/", "* /")
+		s = replaceUntilStable(s, "/*", "/ *")
+		// C and C++ splice a line ending in a backslash (or the ??/ trigraph), optionally followed by
+		// whitespace, with the next line before comments are recognized. This could assemble */ across lines.
+		trimmed := strings.TrimRight(s, " ")
+		if strings.HasSuffix(trimmed, "\\") || strings.HasSuffix(trimmed, "??/") {
+			s = trimmed + "."
+		}
+	case "*)":
+		// Free Pascal nests (* *) comments, so an opening sequence is as dangerous as a closing one.
+		s = replaceUntilStable(s, "*)", "* )")
+		s = replaceUntilStable(s, "(*", "( *")
+	case "'''":
+		// Python headers are string literals, so backslash escapes are interpreted as well.
+		s = strings.ReplaceAll(s, "\\", "\\\\")
+		s = strings.ReplaceAll(s, "'", "\\'")
+	case "\n]]":
+		s = replaceUntilStable(s, "]]", "] ]")
+	}
+	return s
+}
+
+// headerTextNeedsSanitizing returns true if s would be altered in the license header of any generated file
+func headerTextNeedsSanitizing(s string) bool {
+	for _, commentEnd := range []string{"", "*/", "*)", "'''", "\n]]"} {
+		if sanitizeCommentText(s, commentEnd) != s {
+			return true
+		}
+	}
+	return false
+}
+
 // writeLicenseHeaderEx writes a license header into a writer.
 func writeLicenseHeaderEx(w io.Writer, component ComponentDefinition, abstract string, includeVersion bool, CommandStart string, CommandEnd string) {
 	ACTVersion := component.ACTVersion
@@ -162,11 +222,11 @@ func writeLicenseHeaderEx(w io.Writer, component ComponentDefinition, abstract s
 		fmt.Fprintf(w, "%s++\n", CommandStart)
 		fmt.Fprintf(w, "\n")
 	}
-	fmt.Fprintf(w, "Copyright (C) %d %s\n", year, copyright)
+	fmt.Fprintf(w, "Copyright (C) %d %s\n", year, sanitizeCommentText(copyright, CommandEnd))
 	fmt.Fprintf(w, "\n")
 	for i := 0; i < len(component.License.Lines); i++ {
 		line := component.License.Lines[i]
-		fmt.Fprintf(w, "%s\n", line.Value)
+		fmt.Fprintf(w, "%s\n", sanitizeCommentText(line.Value, CommandEnd))
 	}
 	fmt.Fprintf(w, "\n")
 	if includeVersion {