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

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions