Skip to content

Commit c482a5b

Browse files
dcodeIOdcodeIO
committed
Fixed: Unified safe property escapes and added a test for #834 
1 parent e04ddc4 commit c482a5b

22 files changed

+108
-94
lines changed

‎cli/targets/static.js

+4-5
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
module.exports = static_target;
33

44
var protobuf = require("../.."),
5-
cliUtil = require("../util"),
65
UglifyJS = require("uglify-js"),
76
espree = require("espree"),
87
escodegen = require("escodegen"),
@@ -42,7 +41,7 @@ function static_target(root, options, callback) {
4241
}
4342
push("// Exported root namespace");
4443
}
45-
var rootProp = cliUtil.safeProp(config.root || "default");
44+
var rootProp = util.safeProp(config.root || "default");
4645
push((config.es6 ? "const" : "var") + " $root = $protobuf.roots" + rootProp + " || ($protobuf.roots" + rootProp + " = {});");
4746
buildNamespace(null, root);
4847
return callback(null, out.join("\n"));
@@ -98,7 +97,7 @@ function exportName(object, asInterface) {
9897
function escapeName(name) {
9998
if (!name)
10099
return "$root";
101-
return cliUtil.reserved(name) ? name + "_" : name;
100+
return util.isReserved(name) ? name + "_" : name;
102101
}
103102

104103
function aOrAn(name) {
@@ -363,7 +362,7 @@ function buildType(ref, type) {
363362
"@interface " + escapeName("I" + type.name)
364363
];
365364
type.fieldsArray.forEach(function(field) {
366-
var prop = util.safeProp(field.name);
365+
var prop = util.safeProp(field.name); // either .name or ["name"]
367366
prop = prop.substring(1, prop.charAt(0) === "[" ? prop.length - 1 : prop.length);
368367
var jsType = toJsType(field);
369368
if (field.optional)
@@ -398,7 +397,7 @@ function buildType(ref, type) {
398397
jsType = jsType + "|null|undefined";
399398
pushComment([
400399
field.comment || type.name + " " + field.name + ".",
401-
"@member {" + jsType + "} " + escapeName(field.name),
400+
"@member {" + jsType + "} " + field.name,
402401
"@memberof " + exportName(type),
403402
"@instance"
404403
]);

‎cli/util.js

-15
Original file line numberDiff line numberDiff line change
@@ -181,21 +181,6 @@ exports.pad = function(str, len, l) {
181181
return str;
182182
};
183183

184-
exports.reserved = function reserved(name) {
185-
return /^(?:do|if|in|for|let|new|try|var|case|else|enum|eval|false|null|this|true|void|with|break|catch|class|const|super|throw|while|yield|delete|export|import|public|return|static|switch|typeof|default|extends|finally|package|private|continue|debugger|function|arguments|interface|protected|implements|instanceof)$/.test(name);
186-
};
187-
188-
// generate dot-notation property accessors where possible. this saves a few chars (i.e. m.hello
189-
// instead of m["hello"]) but has no measurable performance impact (on V8). not present within the
190-
// library itself because the reserved words check requires a rather longish regex.
191-
exports.safeProp = protobuf.util.safeProp = (function(safeProp) {
192-
return function safeProp_dn(name) {
193-
return !/^[$\w]+$/.test(name) || exports.reserved(name)
194-
? safeProp(name)
195-
: "." + name;
196-
};
197-
})(protobuf.util.safeProp);
198-
199184
exports.jsonSafeProp = function(json) {
200185
return json.replace(/^( +)"(\w+)":/mg, function($0, $1, $2) {
201186
return exports.safeProp($2).charAt(0) === "."

‎dist/light/protobuf.js

+14-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/light/protobuf.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/light/protobuf.min.js

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/light/protobuf.min.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/minimal/protobuf.js

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/minimal/protobuf.min.js

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/protobuf.js

+14-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/protobuf.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/protobuf.min.js

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/protobuf.min.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/descriptor/index.d.ts

+7-24
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,9 @@ export const FileDescriptorSet: $protobuf.Type;
44

55
export const FileDescriptorProto: $protobuf.Type;
66

7-
export const DescriptorProto: $protobuf.Type & {
8-
ExtensionRange: $protobuf.Type,
9-
ReservedRange: $protobuf.Type
10-
};
7+
export const DescriptorProto: $protobuf.Type & { ExtensionRange: $protobuf.Type, ReservedRange: $protobuf.Type};
118

12-
export const FieldDescriptorProto: $protobuf.Type & {
13-
Label: $protobuf.Enum,
14-
Type: $protobuf.Enum
15-
};
9+
export const FieldDescriptorProto: $protobuf.Type & { Label: $protobuf.Enum, Type: $protobuf.Enum};
1610

1711
export const OneofDescriptorProto: $protobuf.Type;
1812

@@ -24,16 +18,11 @@ export const EnumValueDescriptorProto: $protobuf.Type;
2418

2519
export const MethodDescriptorProto: $protobuf.Type;
2620

27-
export const FileOptions: $protobuf.Type & {
28-
OptimizeMode: $protobuf.Enum
29-
};
21+
export const FileOptions: $protobuf.Type & { OptimizeMode: $protobuf.Enum};
3022

3123
export const MessageOptions: $protobuf.Type;
3224

33-
export const FieldOptions: $protobuf.Type & {
34-
CType: $protobuf.Enum,
35-
JSType: $protobuf.Enum
36-
};
25+
export const FieldOptions: $protobuf.Type & { CType: $protobuf.Enum, JSType: $protobuf.Enum};
3726

3827
export const OneofOptions: $protobuf.Type;
3928

@@ -45,17 +34,11 @@ export const ServiceOptions: $protobuf.Type;
4534

4635
export const MethodOptions: $protobuf.Type;
4736

48-
export const UninterpretedOption: $protobuf.Type & {
49-
NamePart: $protobuf.Type
50-
};
37+
export const UninterpretedOption: $protobuf.Type & { NamePart: $protobuf.Type};
5138

52-
export const SourceCodeInfo: $protobuf.Type & {
53-
Location: $protobuf.Type
54-
};
39+
export const SourceCodeInfo: $protobuf.Type & { Location: $protobuf.Type};
5540

56-
export const GeneratedCodeInfo: $protobuf.Type & {
57-
Annotation: $protobuf.Type
58-
};
41+
export const GeneratedCodeInfo: $protobuf.Type & { Annotation: $protobuf.Type};
5942

6043
export interface IFileDescriptorSet {
6144
file: IFileDescriptorProto[];

‎index.d.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -2027,7 +2027,14 @@ export namespace util {
20272027
function toObject(array: any[]): { [k: string]: any };
20282028

20292029
/**
2030-
* Returns a safe property accessor for the specified properly name.
2030+
* Tests whether the specified name is a reserved word in JS.
2031+
* @param name Name to test
2032+
* @returns `true` if reserved, otherwise `false`
2033+
*/
2034+
function isReserved(name: string): boolean;
2035+
2036+
/**
2037+
* Returns a safe property accessor for the specified property name.
20312038
* @param prop Property name
20322039
* @returns Safe accessor
20332040
*/

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
"postinstall": "node scripts/postinstall",
3737
"prof": "node bench/prof",
3838
"test": "tape -r ./lib/tape-adapter tests/*.js tests/node/*.js | tap-spec",
39-
"test-types": "tsc tests/comp_typescript.ts --lib es2015 --strictNullChecks --experimentalDecorators --emitDecoratorMetadata && tsc tests/data/test.ts --lib es2015 --noEmit --strictNullChecks && tsc tests/data/rpc.ts --lib es2015 --noEmit --strictNullChecks",
39+
"test-types": "tsc tests/comp_typescript.ts --lib es2015 --strictNullChecks --experimentalDecorators --emitDecoratorMetadata && tsc tests/data/test.js.ts --lib es2015 --noEmit --strictNullChecks && tsc tests/data/rpc.ts --lib es2015 --noEmit --strictNullChecks",
4040
"types": "node bin/pbts --main --global protobuf --out index.d.ts src/ lib/aspromise/index.js lib/base64/index.js lib/codegen/index.js lib/eventemitter/index.js lib/float/index.js lib/fetch/index.js lib/inquire/index.js lib/path/index.js lib/pool/index.js lib/utf8/index.js && npm run test-types",
4141
"make": "npm run test && npm run types && npm run build && npm run lint",
4242
"release": "npm run make && npm run changelog"

‎src/util.js

+13-2
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,23 @@ var safePropBackslashRe = /\\/g,
5959
safePropQuoteRe = /"/g;
6060

6161
/**
62-
* Returns a safe property accessor for the specified properly name.
62+
* Tests whether the specified name is a reserved word in JS.
63+
* @param {string} name Name to test
64+
* @returns {boolean} `true` if reserved, otherwise `false`
65+
*/
66+
util.isReserved = function isReserved(name) {
67+
return /^(?:do|if|in|for|let|new|try|var|case|else|enum|eval|false|null|this|true|void|with|break|catch|class|const|super|throw|while|yield|delete|export|import|public|return|static|switch|typeof|default|extends|finally|package|private|continue|debugger|function|arguments|interface|protected|implements|instanceof)$/.test(name);
68+
};
69+
70+
/**
71+
* Returns a safe property accessor for the specified property name.
6372
* @param {string} prop Property name
6473
* @returns {string} Safe accessor
6574
*/
6675
util.safeProp = function safeProp(prop) {
67-
return "[\"" + prop.replace(safePropBackslashRe, "\\\\").replace(safePropQuoteRe, "\\\"") + "\"]";
76+
if (!/^[$\w_]+$/.test(prop) || util.isReserved(prop))
77+
return "[\"" + prop.replace(safePropBackslashRe, "\\\\").replace(safePropQuoteRe, "\\\"") + "\"]";
78+
return "." + prop;
6879
};
6980

7081
/**

‎tests/comp_typescript.js

+1-6
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
"use strict";
2-
// uncomment for browser only / non long.js versions
3-
/*
4-
/// <reference path="../stub-long.d.ts" />
5-
/// <reference path="../stub-node.d.ts" />
6-
*/
2+
// test currently consists only of not throwing
73
var __extends = (this && this.__extends) || (function () {
84
var extendStatics = Object.setPrototypeOf ||
95
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
@@ -120,4 +116,3 @@ exports.AwesomeMessage = AwesomeMessage;
120116
var awesomeMessage = new AwesomeMessage({ awesomeField: "hi" });
121117
var awesomeBuffer = AwesomeMessage.encode(awesomeMessage).finish();
122118
var awesomeDecoded = AwesomeMessage.decode(awesomeBuffer);
123-
// test currently consists only of not throwing

‎tests/comp_typescript.ts

+1-7
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
// uncomment for browser only / non long.js versions
2-
/*
3-
/// <reference path="../stub-long.d.ts" />
4-
/// <reference path="../stub-node.d.ts" />
5-
*/
1+
// test currently consists only of not throwing
62

73
import { Root, Message, Type, Field, MapField, OneOf } from "..";
84

@@ -88,5 +84,3 @@ export class AwesomeMessage extends Message<AwesomeMessage> {
8884
let awesomeMessage = new AwesomeMessage({ awesomeField: "hi" });
8985
let awesomeBuffer = AwesomeMessage.encode(awesomeMessage).finish();
9086
let awesomeDecoded = AwesomeMessage.decode(awesomeBuffer);
91-
92-
// test currently consists only of not throwing

‎tests/data/test.d.ts

+8-8
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,9 @@ export namespace jspb {
9595
class SpecialCases implements ISpecialCases {
9696
constructor(properties?: jspb.test.ISpecialCases);
9797
public normal: string;
98-
public default_: string;
99-
public function_: string;
100-
public var_: string;
98+
public default: string;
99+
public function: string;
100+
public var: string;
101101
public static create(properties?: jspb.test.ISpecialCases): jspb.test.SpecialCases;
102102
public static encode(message: jspb.test.ISpecialCases, writer?: $protobuf.Writer): $protobuf.Writer;
103103
public static encodeDelimited(message: jspb.test.ISpecialCases, writer?: $protobuf.Writer): $protobuf.Writer;
@@ -580,10 +580,10 @@ export namespace jspb {
580580
public atwo: number;
581581
public bone: number;
582582
public btwo: number;
583-
public partialOneof?: string;
584-
public recursiveOneof?: string;
585-
public defaultOneofA?: string;
586-
public defaultOneofB?: string;
583+
public partialOneof?: ("pone"|"pthree");
584+
public recursiveOneof?: ("rone"|"rtwo");
585+
public defaultOneofA?: ("aone"|"atwo");
586+
public defaultOneofB?: ("bone"|"btwo");
587587
public static create(properties?: jspb.test.ITestMessageWithOneof): jspb.test.TestMessageWithOneof;
588588
public static encode(message: jspb.test.ITestMessageWithOneof, writer?: $protobuf.Writer): $protobuf.Writer;
589589
public static encodeDelimited(message: jspb.test.ITestMessageWithOneof, writer?: $protobuf.Writer): $protobuf.Writer;
@@ -777,7 +777,7 @@ export namespace google {
777777
class FileDescriptorProto implements IFileDescriptorProto {
778778
constructor(properties?: google.protobuf.IFileDescriptorProto);
779779
public name: string;
780-
public package_: string;
780+
public package: string;
781781
public dependency: string[];
782782
public publicDependency: number[];
783783
public weakDependency: number[];

‎tests/data/test.js

+8-8
Original file line numberDiff line numberDiff line change
@@ -913,23 +913,23 @@ $root.jspb = (function() {
913913

914914
/**
915915
* SpecialCases default.
916-
* @member {string} default_
916+
* @member {string} default
917917
* @memberof jspb.test.SpecialCases
918918
* @instance
919919
*/
920920
SpecialCases.prototype["default"] = "";
921921

922922
/**
923923
* SpecialCases function.
924-
* @member {string} function_
924+
* @member {string} function
925925
* @memberof jspb.test.SpecialCases
926926
* @instance
927927
*/
928928
SpecialCases.prototype["function"] = "";
929929

930930
/**
931931
* SpecialCases var.
932-
* @member {string} var_
932+
* @member {string} var
933933
* @memberof jspb.test.SpecialCases
934934
* @instance
935935
*/
@@ -6142,7 +6142,7 @@ $root.jspb = (function() {
61426142

61436143
/**
61446144
* TestMessageWithOneof partialOneof.
6145-
* @member {string|undefined} partialOneof
6145+
* @member {"pone"|"pthree"|undefined} partialOneof
61466146
* @memberof jspb.test.TestMessageWithOneof
61476147
* @instance
61486148
*/
@@ -6153,7 +6153,7 @@ $root.jspb = (function() {
61536153

61546154
/**
61556155
* TestMessageWithOneof recursiveOneof.
6156-
* @member {string|undefined} recursiveOneof
6156+
* @member {"rone"|"rtwo"|undefined} recursiveOneof
61576157
* @memberof jspb.test.TestMessageWithOneof
61586158
* @instance
61596159
*/
@@ -6164,7 +6164,7 @@ $root.jspb = (function() {
61646164

61656165
/**
61666166
* TestMessageWithOneof defaultOneofA.
6167-
* @member {string|undefined} defaultOneofA
6167+
* @member {"aone"|"atwo"|undefined} defaultOneofA
61686168
* @memberof jspb.test.TestMessageWithOneof
61696169
* @instance
61706170
*/
@@ -6175,7 +6175,7 @@ $root.jspb = (function() {
61756175

61766176
/**
61776177
* TestMessageWithOneof defaultOneofB.
6178-
* @member {string|undefined} defaultOneofB
6178+
* @member {"bone"|"btwo"|undefined} defaultOneofB
61796179
* @memberof jspb.test.TestMessageWithOneof
61806180
* @instance
61816181
*/
@@ -8435,7 +8435,7 @@ $root.google = (function() {
84358435

84368436
/**
84378437
* FileDescriptorProto package.
8438-
* @member {string} package_
8438+
* @member {string} package
84398439
* @memberof google.protobuf.FileDescriptorProto
84408440
* @instance
84418441
*/

0 commit comments

Comments
 (0)