From fb7150eea78d161aac1b304c04531dcd72917720 Mon Sep 17 00:00:00 2001 From: ThePlenkov Date: Tue, 2 Aug 2022 14:50:11 +0000 Subject: [PATCH 1/4] do not allow to extend same field twice to prevent the error --- src/root.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/root.js b/src/root.js index df6f11fa6..aee3423de 100644 --- a/src/root.js +++ b/src/root.js @@ -273,6 +273,10 @@ function tryHandleExtension(root, field) { var extendedType = field.parent.lookup(field.extend); if (extendedType) { var sisterField = new Field(field.fullName, field.id, field.type, field.rule, undefined, field.options); + //do not allow to extend same field twice to prevent the error + if (extendedType.get(sisterField.name)) { + return true; + } sisterField.declaringField = field; field.extensionField = sisterField; extendedType.add(sisterField); From 2b43bdd6ad127e1e7e056373d57529b163538c91 Mon Sep 17 00:00:00 2001 From: ThePlenkov Date: Thu, 18 Aug 2022 08:38:03 +0000 Subject: [PATCH 2/4] Ignore gitpod config --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 6220e292e..b284f56d0 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,4 @@ coverage/ sandbox/ .nyc_output dist/ +.gitpod.yml From 62941d4321824a50d79b7487c1fc1b729002ad0e Mon Sep 17 00:00:00 2001 From: ThePlenkov Date: Thu, 18 Aug 2022 09:55:42 +0000 Subject: [PATCH 3/4] unit test for issue #1783 --- tests/comp_import_extend.js | 18 +++++++++ tests/comp_import_extend.ts | 37 +++++++++++++++++++ tests/data/test-import-extend/main.proto | 6 +++ tests/data/test-import-extend/source_a.proto | 5 +++ tests/data/test-import-extend/source_b.proto | 5 +++ .../data/test-import-extend/validation.proto | 15 ++++++++ 6 files changed, 86 insertions(+) create mode 100644 tests/comp_import_extend.js create mode 100644 tests/comp_import_extend.ts create mode 100644 tests/data/test-import-extend/main.proto create mode 100644 tests/data/test-import-extend/source_a.proto create mode 100644 tests/data/test-import-extend/source_b.proto create mode 100644 tests/data/test-import-extend/validation.proto diff --git a/tests/comp_import_extend.js b/tests/comp_import_extend.js new file mode 100644 index 000000000..30eb40f64 --- /dev/null +++ b/tests/comp_import_extend.js @@ -0,0 +1,18 @@ +"use strict"; +exports.__esModule = true; +var path = require("path"); +var tape = require("tape"); +var protobuf = require("../index"); +// to extend Root +require("../ext/descriptor"); +tape.test("extensions", function (test) { + // load document with extended field imported multiple times + var root = protobuf.loadSync(path.resolve(__dirname, "data/test-import-extend/main.proto")); + root.resolveAll(); + // convert to Descriptor Set + var decodedDescriptorSet = root.toDescriptor("proto3"); + // load back from descriptor set + var root2 = protobuf.Root.fromDescriptor(decodedDescriptorSet); + test.pass("should parse and resolve without errors"); + test.end(); +}); diff --git a/tests/comp_import_extend.ts b/tests/comp_import_extend.ts new file mode 100644 index 000000000..914930a1f --- /dev/null +++ b/tests/comp_import_extend.ts @@ -0,0 +1,37 @@ +import path = require("path"); +import * as tape from "tape"; + +import * as protobuf from "../index"; +import { IFileDescriptorSet } from "../ext/descriptor"; +// to extend Root +require("../ext/descriptor"); + +interface Descriptor { + toDescriptor( + protoVersion: string + ): protobuf.Message & IFileDescriptorSet; + fromDescriptor( + descriptor: IFileDescriptorSet | protobuf.Reader | Uint8Array + ): protobuf.Root; +} + +tape.test("extensions", function (test) { + // load document with extended field imported multiple times + const root = protobuf.loadSync( + path.resolve(__dirname, "data/test-import-extend/main.proto") + ); + root.resolveAll(); + + // convert to Descriptor Set + const decodedDescriptorSet = (root as unknown as Descriptor).toDescriptor( + "proto3" + ); + + // load back from descriptor set + const root2 = (protobuf.Root as unknown as Descriptor).fromDescriptor( + decodedDescriptorSet + ); + + test.pass("should parse and resolve without errors"); + test.end(); +}); diff --git a/tests/data/test-import-extend/main.proto b/tests/data/test-import-extend/main.proto new file mode 100644 index 000000000..d466d3570 --- /dev/null +++ b/tests/data/test-import-extend/main.proto @@ -0,0 +1,6 @@ +syntax = "proto3"; + +package test.import.extend; + +import "source_a.proto"; +import "source_b.proto"; \ No newline at end of file diff --git a/tests/data/test-import-extend/source_a.proto b/tests/data/test-import-extend/source_a.proto new file mode 100644 index 000000000..d6d2a3f88 --- /dev/null +++ b/tests/data/test-import-extend/source_a.proto @@ -0,0 +1,5 @@ +syntax = "proto3"; + +package source.a; + +import "validation.proto"; \ No newline at end of file diff --git a/tests/data/test-import-extend/source_b.proto b/tests/data/test-import-extend/source_b.proto new file mode 100644 index 000000000..f9b7ff66b --- /dev/null +++ b/tests/data/test-import-extend/source_b.proto @@ -0,0 +1,5 @@ +syntax = "proto3"; + +package source.b; + +import "validation.proto"; \ No newline at end of file diff --git a/tests/data/test-import-extend/validation.proto b/tests/data/test-import-extend/validation.proto new file mode 100644 index 000000000..18582955b --- /dev/null +++ b/tests/data/test-import-extend/validation.proto @@ -0,0 +1,15 @@ +syntax = "proto3"; + +package validation; + +import "../google/protobuf/descriptor.proto"; + +option java_package = "com.reserve.validation"; +option java_outer_classname = "ValidationRules"; + +extend google.protobuf.FieldOptions { FieldRules rules = 70001; } + +message FieldRules { + reserved 1; + reserved "optional"; +} \ No newline at end of file From eadbb88f019f133cf72625739b8ff41c581df393 Mon Sep 17 00:00:00 2001 From: ThePlenkov Date: Thu, 18 Aug 2022 11:41:11 +0000 Subject: [PATCH 4/4] using existing test file --- tests/comp_import_extend.js | 4 ++-- tests/comp_import_extend.ts | 4 +--- tests/data/test-import-extend/main.proto | 6 ------ tests/data/test-import-extend/source_a.proto | 5 ----- tests/data/test-import-extend/source_b.proto | 5 ----- tests/data/test-import-extend/validation.proto | 15 --------------- 6 files changed, 3 insertions(+), 36 deletions(-) delete mode 100644 tests/data/test-import-extend/main.proto delete mode 100644 tests/data/test-import-extend/source_a.proto delete mode 100644 tests/data/test-import-extend/source_b.proto delete mode 100644 tests/data/test-import-extend/validation.proto diff --git a/tests/comp_import_extend.js b/tests/comp_import_extend.js index 30eb40f64..f69450369 100644 --- a/tests/comp_import_extend.js +++ b/tests/comp_import_extend.js @@ -1,5 +1,5 @@ "use strict"; -exports.__esModule = true; +Object.defineProperty(exports, "__esModule", { value: true }); var path = require("path"); var tape = require("tape"); var protobuf = require("../index"); @@ -7,7 +7,7 @@ var protobuf = require("../index"); require("../ext/descriptor"); tape.test("extensions", function (test) { // load document with extended field imported multiple times - var root = protobuf.loadSync(path.resolve(__dirname, "data/test-import-extend/main.proto")); + var root = protobuf.loadSync(path.resolve(__dirname, "data/test.proto")); root.resolveAll(); // convert to Descriptor Set var decodedDescriptorSet = root.toDescriptor("proto3"); diff --git a/tests/comp_import_extend.ts b/tests/comp_import_extend.ts index 914930a1f..e25cd224b 100644 --- a/tests/comp_import_extend.ts +++ b/tests/comp_import_extend.ts @@ -17,9 +17,7 @@ interface Descriptor { tape.test("extensions", function (test) { // load document with extended field imported multiple times - const root = protobuf.loadSync( - path.resolve(__dirname, "data/test-import-extend/main.proto") - ); + const root = protobuf.loadSync(path.resolve(__dirname, "data/test.proto")); root.resolveAll(); // convert to Descriptor Set diff --git a/tests/data/test-import-extend/main.proto b/tests/data/test-import-extend/main.proto deleted file mode 100644 index d466d3570..000000000 --- a/tests/data/test-import-extend/main.proto +++ /dev/null @@ -1,6 +0,0 @@ -syntax = "proto3"; - -package test.import.extend; - -import "source_a.proto"; -import "source_b.proto"; \ No newline at end of file diff --git a/tests/data/test-import-extend/source_a.proto b/tests/data/test-import-extend/source_a.proto deleted file mode 100644 index d6d2a3f88..000000000 --- a/tests/data/test-import-extend/source_a.proto +++ /dev/null @@ -1,5 +0,0 @@ -syntax = "proto3"; - -package source.a; - -import "validation.proto"; \ No newline at end of file diff --git a/tests/data/test-import-extend/source_b.proto b/tests/data/test-import-extend/source_b.proto deleted file mode 100644 index f9b7ff66b..000000000 --- a/tests/data/test-import-extend/source_b.proto +++ /dev/null @@ -1,5 +0,0 @@ -syntax = "proto3"; - -package source.b; - -import "validation.proto"; \ No newline at end of file diff --git a/tests/data/test-import-extend/validation.proto b/tests/data/test-import-extend/validation.proto deleted file mode 100644 index 18582955b..000000000 --- a/tests/data/test-import-extend/validation.proto +++ /dev/null @@ -1,15 +0,0 @@ -syntax = "proto3"; - -package validation; - -import "../google/protobuf/descriptor.proto"; - -option java_package = "com.reserve.validation"; -option java_outer_classname = "ValidationRules"; - -extend google.protobuf.FieldOptions { FieldRules rules = 70001; } - -message FieldRules { - reserved 1; - reserved "optional"; -} \ No newline at end of file