fix(schema-compiler): YAML - crash on empty string/null, fix cube-js#…

…7126 (cube-js#7141)
ConsultingMD · Sep 18, 2023 · 73f5ca7 · 73f5ca7
1 parent f5e5372
commit 73f5ca7
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 10 deletions.
diff --git a/packages/cubejs-schema-compiler/src/compiler/YamlCompiler.ts b/packages/cubejs-schema-compiler/src/compiler/YamlCompiler.ts
@@ -140,7 +140,7 @@ export class YamlCompiler {
           } else if (Array.isArray(obj)) {
             const resultAst = t.program([t.expressionStatement(t.arrayExpression(obj.map(code => {
               const ast = this.parsePythonAndTranspileToJs(code, errorsReport);
-              return ast?.body[0]?.expression;
+              return this.extractProgramBodyIfNeeded(ast);
             }).filter(ast => !!ast)))]);
             return this.astIntoArrowFunction(resultAst, '', cubeName);
           }
@@ -153,24 +153,28 @@ export class YamlCompiler {
       return this.astIntoArrowFunction(ast, obj, cubeName, name => this.cubeDictionary.resolveCube(name));
     } else if (typeof obj === 'string') {
       let code = obj;
+
       if (!nonStringFields.has(propertyPath[propertyPath.length - 1])) {
         code = `f"${this.escapeDoubleQuotes(obj)}"`;
       }
+
       const ast = this.parsePythonAndTranspileToJs(code, errorsReport);
-      return ast?.body[0]?.expression;
+      return this.extractProgramBodyIfNeeded(ast);
     } else if (typeof obj === 'boolean') {
       return t.booleanLiteral(obj);
     }
 
-    if (typeof obj === 'object') {
+    if (typeof obj === 'object' && obj !== null) {
       if (Array.isArray(obj)) {
         return t.arrayExpression(obj.map((value, i) => this.transpileYaml(value, propertyPath.concat(i.toString()), cubeName, errorsReport)));
       } else {
         const properties: any[] = [];
+
         for (const propKey of Object.keys(obj)) {
           const ast = this.transpileYaml(obj[propKey], propertyPath.concat(propKey), cubeName, errorsReport);
           properties.push(t.objectProperty(t.stringLiteral(propKey), ast));
         }
+
         return t.objectExpression(properties);
       }
     } else {
@@ -227,26 +231,31 @@ export class YamlCompiler {
     return result.join('');
   }
 
-  private parsePythonIntoArrowFunction(codeString, cubeName, originalObj, errorsReport: ErrorReporter) {
+  private parsePythonIntoArrowFunction(codeString: string, cubeName, originalObj, errorsReport: ErrorReporter) {
     const ast = this.parsePythonAndTranspileToJs(codeString, errorsReport);
-    return this.astIntoArrowFunction(ast, codeString, cubeName);
+    return this.astIntoArrowFunction(ast as any, codeString, cubeName);
   }
 
-  private parsePythonAndTranspileToJs(codeString, errorsReport: ErrorReporter) {
+  private parsePythonAndTranspileToJs(codeString: string, errorsReport: ErrorReporter): t.Program | t.NullLiteral {
+    if (codeString === '' || codeString === 'f""') {
+      return t.nullLiteral();
+    }
+
     try {
       const pythonParser = new PythonParser(codeString);
       return pythonParser.transpileToJs();
     } catch (e: any) {
       errorsReport.error(`Can't parse python expression. Most likely this type of syntax isn't supported yet: ${e.message || e}`);
     }
+
     return t.nullLiteral();
   }
 
-  private astIntoArrowFunction(ast, codeString, cubeName, resolveSymbol?: (string) => any) {
-    const initialJs = babelGenerator(ast, {}, codeString).code;
+  private astIntoArrowFunction(input: t.Program | t.NullLiteral, codeString: string, cubeName, resolveSymbol?: (string) => any) {
+    const initialJs = babelGenerator(input, {}, codeString).code;
 
     // Re-parse generated JS to set all necessary parent paths
-    ast = parse(
+    const ast = parse(
       initialJs,
       {
         sourceType: 'script',
@@ -264,7 +273,8 @@ export class YamlCompiler {
 
     babelTraverse(ast, traverseObj);
 
-    return ast.program.body[0]?.expression;
+    const body: any = ast.program.body[0];
+    return body?.expression;
   }
 
   private yamlArrayToObj(yamlArray, memberType: string, errorsReport: ErrorReporter) {
@@ -289,4 +299,13 @@ export class YamlCompiler {
 
     return remapped.reduce((a, b) => ({ ...a, ...b }), {});
   }
+
+  private extractProgramBodyIfNeeded(ast: t.Node) {
+    if (t.isProgram(ast)) {
+      const body: any = ast?.body[0];
+      return body?.expression;
+    }
+
+    return ast;
+  }
 }
diff --git a/packages/cubejs-schema-compiler/test/unit/yaml-schema.test.ts b/packages/cubejs-schema-compiler/test/unit/yaml-schema.test.ts
@@ -57,4 +57,36 @@ describe('Yaml Schema Testing', () => {
 
     await compiler.compile();
   });
+
+  it('empty string - issue#7126', async () => {
+    const { compiler } = prepareYamlCompiler(
+      `cubes:
+  - name: Users
+    title: ''`
+    );
+
+    try {
+      await compiler.compile();
+
+      throw new Error('compile must return an error');
+    } catch (e: any) {
+      expect(e.message).toContain('Users cube: (title = null) must be a string');
+    }
+  });
+
+  it('null for string field', async () => {
+    const { compiler } = prepareYamlCompiler(
+      `cubes:
+  - name: Users
+    title: null`
+    );
+
+    try {
+      await compiler.compile();
+
+      throw new Error('compile must return an error');
+    } catch (e: any) {
+      expect(e.message).toContain('Unexpected input during yaml transpiling: null');
+    }
+  });
 });