Skip to content

bug: Protobuf verification bugs (generated output fails protobufjs) #249

Description

@apoorv7g

Generated Proto3 from Concerto CTO models is written by ProtobufVisitor (lib/codegen/fromcto/protobuf/protobufvisitor.js). Today, npm test only checks snapshots, it does not run a Proto3 parser on the output.

When you compile with protobufjs parse(), standard repo fixtures produce invalid .proto files.

Related: bug: GraphQL verification bugs (generated output fails graphql-js) #248


Bug 1: optional map<...> in hr.cto

Cause

hr.cto defines optional map fields on concepts such as Company:

concept Company {
    ...
    o CompanyProperties companyProperties optional
    ...
}

ProtobufVisitor emits optional before map<...> field definitions. In Proto3, map fields cannot use the optional label - map entries are already implicitly optional.

Reproduce

From the repo root (after npm ci):

node -e "
const fs = require('fs');
const path = require('path');
const protobuf = require('protobufjs');
const { dir } = require('tmp-promise');
const { ModelManager } = require('@accordproject/concerto-core');
const { FileWriter } = require('@accordproject/concerto-util');
const ProtobufVisitor = require('./lib/codegen/fromcto/protobuf/protobufvisitor.js');

process.env.ENABLE_MAP_TYPE = 'true';
process.env.IMPORT_ALIASING = 'true';

const MODEL_DIR = './test/codegen/fromcto/data/model';
const GOOGLE_PROTO_ROOT = path.dirname(require.resolve('protobufjs/package.json'));

(async () => {
  const mm = new ModelManager();
  mm.addCTOModel(fs.readFileSync(path.join(MODEL_DIR, 'hr_base.cto'), 'utf8'), 'hr_base.cto');
  mm.addCTOModel(fs.readFileSync(path.join(MODEL_DIR, 'hr.cto'), 'utf8'), 'hr.cto');

  const { path: out, cleanup } = await dir({ unsafeCleanup: true });
  mm.accept(new ProtobufVisitor(), {
    fileWriter: new FileWriter(out),
    showCompositionRelationships: true,
  });

  const root = new protobuf.Root();
  root.resolvePath = (origin, target) => {
    if (target.startsWith('google/protobuf/')) {
      return path.join(GOOGLE_PROTO_ROOT, target);
    }
    return path.join(out, path.basename(target));
  };

  const proto = fs.readFileSync(path.join(out, 'org.acme.hr.v1_0_0.proto'), 'utf8');
  protobuf.parse(proto, root, 'org.acme.hr.v1_0_0.proto');
  console.log('OK');
  await cleanup();
})().catch((err) => {
  console.error(err.message);
  process.exit(1);
});
"

Expected error

illegal name '<' (line 19)

Generated fragment

message Company {
  optional map<string, string> CompanyProperties = 1;
  optional map<SSN, Employee> EmployeeDirectory = 2;
  ...
}

Bug 2: Custom scalar map keys (e.g. SSN) in hr_base.cto + hr.cto

Cause

hr_base.cto defines a custom scalar SSN. hr.cto uses it as a map key in declarations such as EmployeeDirectory and EmployeeTShirtSizes.

Proto3 only allows integral types (int32, int64, uint32, uint64, sint32, sint64, fixed32, fixed64, sfixed32, sfixed64) and bool/string as map key types. A custom scalar like SSN is not valid.

Reproduce

Written for powershell

@'
const protobuf = require("protobufjs");
const proto = [
  'syntax = "proto3";',
  'message SSN {}',
  'message Employee {}',
  'message Company {',
  '  map<SSN, Employee> EmployeeDirectory = 1;',
  '}',
].join("\n");
try {
  protobuf.parse(proto);
  console.log("OK");
} catch (err) {
  console.error(err.message);
  process.exit(1);
}
'@ | node

Expected error

illegal type 'SSN' (line 5)

Generated references

optional map<SSN, Employee> EmployeeDirectory = 2;
optional map<SSN, TShirtSizeType> EmployeeTShirtSizes = 5;
map<SSN, KinTelephone> NextOfKin = 12;

Related code

File Role
lib/codegen/fromcto/protobuf/protobufvisitor.js Emits Proto3 (visitMapDeclaration, optional field rules)
test/codegen/codegen.js Snapshot tests (no Proto3 parser on output)
test/codegen/fromcto/protobuf/protobufvisitor.js Unit tests with mocks, not full HR fixtures

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions