github-code-scanning[bot] commented on code in PR #1833: URL: https://github.com/apache/avro/pull/1833#discussion_r950928308
########## lang/csharp/src/apache/test/IO/JsonCodecTests.cs: ########## @@ -0,0 +1,226 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; +using NUnit.Framework; +using System.IO; +using System.Text; +using Avro.Generic; +using Avro.IO; +using Newtonsoft.Json.Linq; + +namespace Avro.Test +{ + using Decoder = Avro.IO.Decoder; + using Encoder = Avro.IO.Encoder; + + /// <summary> + /// Tests the JsonEncoder and JsonDecoder. + /// </summary> + [TestFixture] + public class JsonCodecTests + { + [TestCase] + public void TestJsonEncoderWhenIncludeNamespaceOptionIsFalse() + { + string value = "{\"b\": {\"string\":\"myVal\"}, \"a\": 1}"; + string schemaStr = "{\"type\": \"record\", \"name\": \"ab\", \"fields\": [" + + "{\"name\": \"a\", \"type\": \"int\"}, {\"name\": \"b\", \"type\": [\"null\", \"string\"]}" + + "]}"; + Schema schema = Schema.Parse(schemaStr); + byte[] avroBytes = fromJsonToAvro(value, schema); + + Assert.IsTrue(JToken.DeepEquals(JObject.Parse("{\"b\":\"myVal\",\"a\":1}"), + JObject.Parse(fromAvroToJson(avroBytes, schema, false)))); + } + + [TestCase] + public void TestJsonEncoderWhenIncludeNamespaceOptionIsTrue() + { + string value = "{\"b\": {\"string\":\"myVal\"}, \"a\": 1}"; + string schemaStr = "{\"type\": \"record\", \"name\": \"ab\", \"fields\": [" + + "{\"name\": \"a\", \"type\": \"int\"}, {\"name\": \"b\", \"type\": [\"null\", \"string\"]}" + + "]}"; + Schema schema = Schema.Parse(schemaStr); + byte[] avroBytes = fromJsonToAvro(value, schema); + + Assert.IsTrue(JToken.DeepEquals(JObject.Parse("{\"b\":{\"string\":\"myVal\"},\"a\":1}"), + JObject.Parse(fromAvroToJson(avroBytes, schema, true)))); + } + + [TestCase] + public void TestJsonRecordOrdering() + { + string value = "{\"b\": 2, \"a\": 1}"; + Schema schema = Schema.Parse("{\"type\": \"record\", \"name\": \"ab\", \"fields\": [" + + "{\"name\": \"a\", \"type\": \"int\"}, {\"name\": \"b\", \"type\": \"int\"}" + + "]}"); + GenericDatumReader<object> reader = new GenericDatumReader<object>(schema, schema); + Decoder decoder = new JsonDecoder(schema, value); + object o = reader.Read(null, decoder); + + Assert.AreEqual("{\"a\":1,\"b\":2}", fromDatumToJson(o, schema, false)); + } + + [TestCase] + public void TestJsonRecordOrdering2() + { + string value = "{\"b\": { \"b3\": 1.4, \"b2\": 3.14, \"b1\": \"h\"}, \"a\": {\"a2\":true, \"a1\": null}}"; + Schema schema = Schema.Parse("{\"type\": \"record\", \"name\": \"ab\", \"fields\": [\n" + + "{\"name\": \"a\", \"type\": {\"type\":\"record\",\"name\":\"A\",\"fields\":\n" + + "[{\"name\":\"a1\", \"type\":\"null\"}, {\"name\":\"a2\", \"type\":\"boolean\"}]}},\n" + + "{\"name\": \"b\", \"type\": {\"type\":\"record\",\"name\":\"B\",\"fields\":\n" + + "[{\"name\":\"b1\", \"type\":\"string\"}, {\"name\":\"b2\", \"type\":\"float\"}, {\"name\":\"b3\", \"type\":\"double\"}]}}\n" + + "]}"); + GenericDatumReader<object> reader = new GenericDatumReader<object>(schema, schema); + Decoder decoder = new JsonDecoder(schema, value); + object o = reader.Read(null, decoder); + + Assert.AreEqual("{\"a\":{\"a1\":null,\"a2\":true},\"b\":{\"b1\":\"h\",\"b2\":3.14,\"b3\":1.4}}", + fromDatumToJson(o, schema, false)); + } + + [TestCase] + public void TestJsonRecordOrderingWithProjection() + { + String value = "{\"b\": { \"b3\": 1.4, \"b2\": 3.14, \"b1\": \"h\"}, \"a\": {\"a2\":true, \"a1\": null}}"; + Schema writerSchema = Schema.Parse("{\"type\": \"record\", \"name\": \"ab\", \"fields\": [\n" + + "{\"name\": \"a\", \"type\": {\"type\":\"record\",\"name\":\"A\",\"fields\":\n" + + "[{\"name\":\"a1\", \"type\":\"null\"}, {\"name\":\"a2\", \"type\":\"boolean\"}]}},\n" + + "{\"name\": \"b\", \"type\": {\"type\":\"record\",\"name\":\"B\",\"fields\":\n" + + "[{\"name\":\"b1\", \"type\":\"string\"}, {\"name\":\"b2\", \"type\":\"float\"}, {\"name\":\"b3\", \"type\":\"double\"}]}}\n" + + "]}"); + Schema readerSchema = Schema.Parse("{\"type\": \"record\", \"name\": \"ab\", \"fields\": [\n" + + "{\"name\": \"a\", \"type\": {\"type\":\"record\",\"name\":\"A\",\"fields\":\n" + + "[{\"name\":\"a1\", \"type\":\"null\"}, {\"name\":\"a2\", \"type\":\"boolean\"}]}}\n" + + "]}"); + GenericDatumReader<object> reader = new GenericDatumReader<object>(writerSchema, readerSchema); + Decoder decoder = new JsonDecoder(writerSchema, value); + Object o = reader.Read(null, decoder); + + Assert.AreEqual("{\"a\":{\"a1\":null,\"a2\":true}}", + fromDatumToJson(o, readerSchema, false)); + } + + + [TestCase] + public void testJsonRecordOrderingWithProjection2() + { + String value = + "{\"b\": { \"b1\": \"h\", \"b2\": [3.14, 3.56], \"b3\": 1.4}, \"a\": {\"a2\":true, \"a1\": null}}"; + Schema writerSchema = Schema.Parse("{\"type\": \"record\", \"name\": \"ab\", \"fields\": [\n" + + "{\"name\": \"a\", \"type\": {\"type\":\"record\",\"name\":\"A\",\"fields\":\n" + + "[{\"name\":\"a1\", \"type\":\"null\"}, {\"name\":\"a2\", \"type\":\"boolean\"}]}},\n" + + "{\"name\": \"b\", \"type\": {\"type\":\"record\",\"name\":\"B\",\"fields\":\n" + + "[{\"name\":\"b1\", \"type\":\"string\"}, {\"name\":\"b2\", \"type\":{\"type\":\"array\", \"items\":\"float\"}}, {\"name\":\"b3\", \"type\":\"double\"}]}}\n" + + "]}"); + + Schema readerSchema = Schema.Parse("{\"type\": \"record\", \"name\": \"ab\", \"fields\": [\n" + + "{\"name\": \"a\", \"type\": {\"type\":\"record\",\"name\":\"A\",\"fields\":\n" + + "[{\"name\":\"a1\", \"type\":\"null\"}, {\"name\":\"a2\", \"type\":\"boolean\"}]}}\n" + + "]}"); + + GenericDatumReader<object> reader = new GenericDatumReader<object>(writerSchema, readerSchema); + Decoder decoder = new JsonDecoder(writerSchema, value); + object o = reader.Read(null, decoder); + + Assert.AreEqual("{\"a\":{\"a1\":null,\"a2\":true}}", + fromDatumToJson(o, readerSchema, false)); + } + + [TestCase("int", 1)] + [TestCase("long", 1L)] + [TestCase("float", 1.0F)] + [TestCase("double", 1.0)] + public void TestJsonDecoderNumeric(string type, object value) + { + string def = "{\"type\":\"record\",\"name\":\"X\",\"fields\":" + "[{\"type\":\"" + type + + "\",\"name\":\"n\"}]}"; + Schema schema = Schema.Parse(def); + DatumReader<GenericRecord> reader = new GenericDatumReader<GenericRecord>(schema, schema); + + string[] records = { "{\"n\":1}", "{\"n\":1.0}" }; + + foreach (string record in records) + { + Decoder decoder = new JsonDecoder(schema, record); + GenericRecord r = reader.Read(null, decoder); + Assert.AreEqual(value, r["n"]); + } Review Comment: ## Missed opportunity to use Select This foreach loop immediately maps its iteration variable to another variable [here](1) - consider mapping the sequence explicitly using '.Select(...)'. [Show more details](https://github.com/apache/avro/security/code-scanning/2897) ########## lang/csharp/src/apache/main/IO/Parsing/Symbol.cs: ########## @@ -0,0 +1,778 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; + +namespace Avro.IO.Parsing +{ + /// <summary> + /// Symbol is the base of all symbols (terminals and non-terminals) of the + /// grammar. + /// </summary> + public abstract class Symbol + { + /// <summary> + /// The type of symbol. + /// </summary> + public enum Kind + { + /// <summary> + /// terminal symbols which have no productions </summary> + Terminal, + + /// <summary> + /// Start symbol for some grammar </summary> + Root, + + /// <summary> + /// non-terminal symbol which is a sequence of one or more other symbols </summary> + Sequence, + + /// <summary> + /// non-terminal to represent the contents of an array or map </summary> + Repeater, + + /// <summary> + /// non-terminal to represent the union </summary> + Alternative, + + /// <summary> + /// non-terminal action symbol which are automatically consumed </summary> + ImplicitAction, + + /// <summary> + /// non-terminal action symbol which is explicitly consumed </summary> + ExplicitAction + } + + /// The kind of this symbol. + public readonly Kind SymKind; + + /// <summary> + /// The production for this symbol. If this symbol is a terminal this is + /// <tt>null</tt>. Otherwise this holds the the sequence of the symbols that + /// forms the production for this symbol. The sequence is in the reverse order of + /// production. This is useful for easy copying onto parsing stack. + /// + /// Please note that this is a final. So the production for a symbol should be + /// known before that symbol is constructed. This requirement cannot be met for + /// those symbols which are recursive (e.g. a record that holds union a branch of + /// which is the record itself). To resolve this problem, we initialize the + /// symbol with an array of nulls. Later we fill the symbols. Not clean, but + /// works. The other option is to not have this field a final. But keeping it + /// final and thus keeping symbol immutable gives some comfort. See various + /// generators how we generate records. + /// </summary> + public readonly Symbol[] Production; + + /// <summary> + /// Constructs a new symbol of the given kind. + /// </summary> + protected Symbol(Kind kind) : this(kind, null) + { + } + + /// <summary> + /// Constructs a new symbol of the given kind and production. + /// </summary> + protected Symbol(Kind kind, Symbol[] production) + { + this.Production = production; + this.SymKind = kind; + } + + /// <summary> + /// A convenience method to construct a root symbol. + /// </summary> + public static Symbol NewRoot(params Symbol[] symbols) + { + return new Root(symbols); + } + + /// <summary> + /// A convenience method to construct a sequence. + /// </summary> + /// <param name="production"> The constituent symbols of the sequence. </param> + public static Symbol NewSeq(params Symbol[] production) + { + return new Sequence(production); + } + + /// <summary> + /// A convenience method to construct a repeater. + /// </summary> + /// <param name="endSymbol"> The end symbol. </param> + /// <param name="symsToRepeat"> The symbols to repeat in the repeater. </param> + public static Symbol NewRepeat(Symbol endSymbol, params Symbol[] symsToRepeat) + { + return new Repeater(endSymbol, symsToRepeat); + } + + /// <summary> + /// A convenience method to construct a union. + /// </summary> + public static Symbol NewAlt(Symbol[] symbols, string[] labels) + { + return new Alternative(symbols, labels); + } + + /// <summary> + /// A convenience method to construct an ErrorAction. + /// </summary> + /// <param name="e"> </param> + protected static Symbol Error(string e) + { + return new ErrorAction(e); + } + + /// <summary> + /// A convenience method to construct a ResolvingAction. + /// </summary> + /// <param name="w"> The writer symbol </param> + /// <param name="r"> The reader symbol </param> + protected static Symbol Resolve(Symbol w, Symbol r) + { + return new ResolvingAction(w, r); + } + + protected class Fixup + { + public readonly Symbol[] Symbols; + public readonly int Pos; + + public Fixup(Symbol[] symbols, int pos) + { + this.Symbols = symbols; + this.Pos = pos; + } + } + + protected virtual Symbol Flatten(IDictionary<Sequence, Sequence> map, IDictionary<Sequence, IList<Fixup>> map2) + { + return this; + } + + public virtual int FlattenedSize() + { + return 1; + } + + /// <summary> + /// Flattens the given sub-array of symbols into an sub-array of symbols. Every + /// <tt>Sequence</tt> in the input are replaced by its production recursively. + /// Non-<tt>Sequence</tt> symbols, they internally have other symbols those + /// internal symbols also get flattened. When flattening is done, the only place + /// there might be Sequence symbols is in the productions of a Repeater, + /// Alternative, or the symToParse and symToSkip in a UnionAdjustAction or + /// SkipAction. + /// + /// Why is this done? We want our parsers to be fast. If we left the grammars + /// unflattened, then the parser would be constantly copying the contents of + /// nested Sequence productions onto the parsing stack. Instead, because of + /// flattening, we have a long top-level production with no Sequences unless the + /// Sequence is absolutely needed, e.g., in the case of a Repeater or an + /// Alternative. + /// + /// Well, this is not exactly true when recursion is involved. Where there is a + /// recursive record, that record will be "inlined" once, but any internal (ie, + /// recursive) references to that record will be a Sequence for the record. That + /// Sequence will not further inline itself -- it will refer to itself as a + /// Sequence. The same is true for any records nested in this outer recursive + /// record. Recursion is rare, and we want things to be fast in the typical case, + /// which is why we do the flattening optimization. + /// + /// + /// The algorithm does a few tricks to handle recursive symbol definitions. In + /// order to avoid infinite recursion with recursive symbols, we have a map of + /// Symbol->Symbol. Before fully constructing a flattened symbol for a + /// <tt>Sequence</tt> we insert an empty output symbol into the map and then + /// start filling the production for the <tt>Sequence</tt>. If the same + /// <tt>Sequence</tt> is encountered due to recursion, we simply return the + /// (empty) output <tt>Sequence</tt> from the map. Then we actually fill out + /// the production for the <tt>Sequence</tt>. As part of the flattening process + /// we copy the production of <tt>Sequence</tt>s into larger arrays. If the + /// original <tt>Sequence</tt> has not not be fully constructed yet, we copy a + /// bunch of <tt>null</tt>s. Fix-up remembers all those <tt>null</tt> patches. + /// The fix-ups gets finally filled when we know the symbols to occupy those + /// patches. + /// </summary> + /// <param name="in"> The array of input symbols to flatten </param> + /// <param name="start"> The position where the input sub-array starts. </param> + /// <param name="out"> The output that receives the flattened list of symbols. The + /// output array should have sufficient space to receive the + /// expanded sub-array of symbols. </param> + /// <param name="skip"> The position where the output input sub-array starts. </param> + /// <param name="map"> A map of symbols which have already been expanded. Useful for + /// handling recursive definitions and for caching. </param> + /// <param name="map2"> A map to to store the list of fix-ups. </param> + protected static void Flatten(Symbol[] @in, int start, Symbol[] @out, int skip, + IDictionary<Sequence, Sequence> map, IDictionary<Sequence, IList<Fixup>> map2) + { + for (int i = start, j = skip; i < @in.Length; i++) + { + Symbol s = @in[i].Flatten(map, map2); + if (s is Sequence) + { + Symbol[] p = s.Production; + IList<Fixup> l = map2.ContainsKey((Sequence)s) ? map2[(Sequence)s] : null; + if (l == null) + { + Array.Copy(p, 0, @out, j, p.Length); + // Copy any fixups that will be applied to p to add missing symbols + foreach (IList<Fixup> fixups in map2.Values) + { + copyFixups(fixups, @out, j, p); + } + } + else + { + l.Add(new Fixup(@out, j)); + } + + j += p.Length; + } + else + { + @out[j++] = s; + } + } + } + + private static void copyFixups(IList<Fixup> fixups, Symbol[] @out, int outPos, Symbol[] toCopy) + { + for (int i = 0, n = fixups.Count; i < n; i += 1) + { + Fixup fixup = fixups[i]; + if (fixup.Symbols == toCopy) + { + fixups.Add(new Fixup(@out, fixup.Pos + outPos)); + } + } + } + + /// <summary> + /// Returns the amount of space required to flatten the given sub-array of + /// symbols. + /// </summary> + /// <param name="symbols"> The array of input symbols. </param> + /// <param name="start"> The index where the subarray starts. </param> + /// <returns> The number of symbols that will be produced if one expands the given + /// input. </returns> + protected static int FlattenedSize(Symbol[] symbols, int start) + { + int result = 0; + for (int i = start; i < symbols.Length; i++) + { + if (symbols[i] is Sequence) + { + Sequence s = (Sequence)symbols[i]; + result += s.FlattenedSize(); + } + else + { + result += 1; + } + } + + return result; + } + + protected class Terminal : Symbol + { + public readonly string PrintName; + + public Terminal(string printName) : base(Kind.Terminal) + { + this.PrintName = printName; + } + + public override string ToString() + { + return PrintName; + } + } + + public class ImplicitAction : Symbol + { + /// <summary> + /// Set to <tt>true</tt> if and only if this implicit action is a trailing + /// action. That is, it is an action that follows real symbol. E.g + /// <seealso cref="Symbol.DefaultEndAction"/>. + /// </summary> + public readonly bool IsTrailing; + + public ImplicitAction() : this(false) + { + } + + public ImplicitAction(bool isTrailing) : base(Kind.ImplicitAction) + { + this.IsTrailing = isTrailing; + } + } + + protected class Root : Symbol + { + public Root(params Symbol[] symbols) : base(Kind.Root, makeProduction(symbols)) + { + Production[0] = this; + } + + private static Symbol[] makeProduction(Symbol[] symbols) + { + Symbol[] result = new Symbol[FlattenedSize(symbols, 0) + 1]; + Flatten(symbols, 0, result, 1, new Dictionary<Sequence, Sequence>(), + new Dictionary<Sequence, IList<Fixup>>()); + return result; + } + } + + protected class Sequence : Symbol, IEnumerable<Symbol> + { + public Sequence(Symbol[] productions) : base(Kind.Sequence, productions) + { + } + + public virtual Symbol Get(int index) + { + return Production[index]; + } + + public virtual int Size() + { + return Production.Length; + } + + public IEnumerator<Symbol> GetEnumerator() + { + return Enumerable.Reverse(Production).GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return this.GetEnumerator(); + } + + protected override Symbol Flatten(IDictionary<Sequence, Sequence> map, + IDictionary<Sequence, IList<Fixup>> map2) + { + Sequence result = map.ContainsKey(this) ? map[this] : null; Review Comment: ## Inefficient use of ContainsKey Inefficient use of 'ContainsKey' and [indexer](1). [Show more details](https://github.com/apache/avro/security/code-scanning/2900) ########## lang/csharp/src/apache/main/IO/Parsing/Symbol.cs: ########## @@ -0,0 +1,778 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; + +namespace Avro.IO.Parsing +{ + /// <summary> + /// Symbol is the base of all symbols (terminals and non-terminals) of the + /// grammar. + /// </summary> + public abstract class Symbol + { + /// <summary> + /// The type of symbol. + /// </summary> + public enum Kind + { + /// <summary> + /// terminal symbols which have no productions </summary> + Terminal, + + /// <summary> + /// Start symbol for some grammar </summary> + Root, + + /// <summary> + /// non-terminal symbol which is a sequence of one or more other symbols </summary> + Sequence, + + /// <summary> + /// non-terminal to represent the contents of an array or map </summary> + Repeater, + + /// <summary> + /// non-terminal to represent the union </summary> + Alternative, + + /// <summary> + /// non-terminal action symbol which are automatically consumed </summary> + ImplicitAction, + + /// <summary> + /// non-terminal action symbol which is explicitly consumed </summary> + ExplicitAction + } + + /// The kind of this symbol. + public readonly Kind SymKind; + + /// <summary> + /// The production for this symbol. If this symbol is a terminal this is + /// <tt>null</tt>. Otherwise this holds the the sequence of the symbols that + /// forms the production for this symbol. The sequence is in the reverse order of + /// production. This is useful for easy copying onto parsing stack. + /// + /// Please note that this is a final. So the production for a symbol should be + /// known before that symbol is constructed. This requirement cannot be met for + /// those symbols which are recursive (e.g. a record that holds union a branch of + /// which is the record itself). To resolve this problem, we initialize the + /// symbol with an array of nulls. Later we fill the symbols. Not clean, but + /// works. The other option is to not have this field a final. But keeping it + /// final and thus keeping symbol immutable gives some comfort. See various + /// generators how we generate records. + /// </summary> + public readonly Symbol[] Production; + + /// <summary> + /// Constructs a new symbol of the given kind. + /// </summary> + protected Symbol(Kind kind) : this(kind, null) + { + } + + /// <summary> + /// Constructs a new symbol of the given kind and production. + /// </summary> + protected Symbol(Kind kind, Symbol[] production) + { + this.Production = production; + this.SymKind = kind; + } + + /// <summary> + /// A convenience method to construct a root symbol. + /// </summary> + public static Symbol NewRoot(params Symbol[] symbols) + { + return new Root(symbols); + } + + /// <summary> + /// A convenience method to construct a sequence. + /// </summary> + /// <param name="production"> The constituent symbols of the sequence. </param> + public static Symbol NewSeq(params Symbol[] production) + { + return new Sequence(production); + } + + /// <summary> + /// A convenience method to construct a repeater. + /// </summary> + /// <param name="endSymbol"> The end symbol. </param> + /// <param name="symsToRepeat"> The symbols to repeat in the repeater. </param> + public static Symbol NewRepeat(Symbol endSymbol, params Symbol[] symsToRepeat) + { + return new Repeater(endSymbol, symsToRepeat); + } + + /// <summary> + /// A convenience method to construct a union. + /// </summary> + public static Symbol NewAlt(Symbol[] symbols, string[] labels) + { + return new Alternative(symbols, labels); + } + + /// <summary> + /// A convenience method to construct an ErrorAction. + /// </summary> + /// <param name="e"> </param> + protected static Symbol Error(string e) + { + return new ErrorAction(e); + } + + /// <summary> + /// A convenience method to construct a ResolvingAction. + /// </summary> + /// <param name="w"> The writer symbol </param> + /// <param name="r"> The reader symbol </param> + protected static Symbol Resolve(Symbol w, Symbol r) + { + return new ResolvingAction(w, r); + } + + protected class Fixup + { + public readonly Symbol[] Symbols; + public readonly int Pos; + + public Fixup(Symbol[] symbols, int pos) Review Comment: ## Exposing internal representation 'Fixup' exposes the internal representation stored in field 'Symbols'. The value may be modified [through the variable out](1). 'Fixup' exposes the internal representation stored in field 'Symbols'. The value may be modified [through the variable out](2). 'Fixup' exposes the internal representation stored in field 'Symbols'. The value may be modified [through the variable out](3). 'Fixup' exposes the internal representation stored in field 'Symbols'. The value may be modified [through the variable out](4). [Show more details](https://github.com/apache/avro/security/code-scanning/2895) ########## lang/csharp/src/apache/main/IO/Parsing/JsonGrammarGenerator.cs: ########## @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; +using System.Collections.Generic; + +namespace Avro.IO.Parsing +{ + /// <summary> + /// The class that generates a grammar suitable to parse Avro data in JSON + /// format. + /// </summary> + public class JsonGrammarGenerator : ValidatingGrammarGenerator + { + /// <summary> + /// Returns the non-terminal that is the start symbol for the grammar for the + /// grammar for the given schema <tt>sc</tt>. + /// </summary> + public override Symbol Generate(Schema schema) + { + return Symbol.NewRoot(Generate(schema, new Dictionary<LitS, Symbol>())); + } + + /// <summary> + /// Returns the non-terminal that is the start symbol for grammar of the given + /// schema <tt>sc</tt>. If there is already an entry for the given schema in the + /// given map <tt>seen</tt> then that entry is returned. Otherwise a new symbol + /// is generated and an entry is inserted into the map. + /// </summary> + /// <param name="sc"> The schema for which the start symbol is required </param> + /// <param name="seen"> A map of schema to symbol mapping done so far. </param> + /// <returns> The start symbol for the schema </returns> + protected override Symbol Generate(Schema sc, IDictionary<LitS, Symbol> seen) + { + switch (sc.Tag) + { + case Schema.Type.Null: + case Schema.Type.Boolean: + case Schema.Type.Int: + case Schema.Type.Long: + case Schema.Type.Float: + case Schema.Type.Double: + case Schema.Type.String: + case Schema.Type.Bytes: + case Schema.Type.Fixed: + case Schema.Type.Union: + return base.Generate(sc, seen); + case Schema.Type.Enumeration: + return Symbol.NewSeq(new Symbol.EnumLabelsAction(((EnumSchema)sc).Symbols), Symbol.Enum); + case Schema.Type.Array: + return Symbol.NewSeq( + Symbol.NewRepeat(Symbol.ArrayEnd, Symbol.ItemEnd, Generate(((ArraySchema)sc).ItemSchema, seen)), + Symbol.ArrayStart); + case Schema.Type.Map: + return Symbol.NewSeq( + Symbol.NewRepeat(Symbol.MapEnd, Symbol.ItemEnd, Generate(((MapSchema)sc).ValueSchema, seen), + Symbol.MapKeyMarker, Symbol.String), Symbol.MapStart); + case Schema.Type.Record: + { + LitS wsc = new LitS(sc); + Symbol rresult = seen.ContainsKey(wsc) ? seen[wsc] : null; Review Comment: ## Inefficient use of ContainsKey Inefficient use of 'ContainsKey' and [indexer](1). [Show more details](https://github.com/apache/avro/security/code-scanning/2898) ########## lang/csharp/src/apache/main/IO/Parsing/ValidatingGrammarGenerator.cs: ########## @@ -0,0 +1,152 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; +using System.Collections.Generic; + +namespace Avro.IO.Parsing +{ + /// <summary> + /// The class that generates validating grammar. + /// </summary> + public class ValidatingGrammarGenerator + { + /// <summary> + /// Returns the non-terminal that is the start symbol for the grammar for the + /// given schema <tt>sc</tt>. + /// </summary> + public virtual Symbol Generate(Schema schema) + { + return Symbol.NewRoot(Generate(schema, new Dictionary<LitS, Symbol>())); + } + + /// <summary> + /// Returns the non-terminal that is the start symbol for the grammar for the + /// given schema <tt>sc</tt>. If there is already an entry for the given schema + /// in the given map <tt>seen</tt> then that entry is returned. Otherwise a new + /// symbol is generated and an entry is inserted into the map. + /// </summary> + /// <param name="sc"> The schema for which the start symbol is required </param> + /// <param name="seen"> A map of schema to symbol mapping done so far. </param> + /// <returns> The start symbol for the schema </returns> + protected virtual Symbol Generate(Schema sc, IDictionary<LitS, Symbol> seen) + { + switch (sc.Tag) + { + case Schema.Type.Null: + return Symbol.Null; + case Schema.Type.Boolean: + return Symbol.Boolean; + case Schema.Type.Int: + return Symbol.Int; + case Schema.Type.Long: + return Symbol.Long; + case Schema.Type.Float: + return Symbol.Float; + case Schema.Type.Double: + return Symbol.Double; + case Schema.Type.String: + return Symbol.String; + case Schema.Type.Bytes: + return Symbol.Bytes; + case Schema.Type.Fixed: + return Symbol.NewSeq(new Symbol.IntCheckAction(((FixedSchema)sc).Size), Symbol.Fixed); + case Schema.Type.Enumeration: + return Symbol.NewSeq(new Symbol.IntCheckAction(((EnumSchema)sc).Symbols.Count), Symbol.Enum); + case Schema.Type.Array: + return Symbol.NewSeq( + Symbol.NewRepeat(Symbol.ArrayEnd, Generate(((ArraySchema)sc).ItemSchema, seen)), + Symbol.ArrayStart); + case Schema.Type.Map: + return Symbol.NewSeq( + Symbol.NewRepeat(Symbol.MapEnd, Generate(((MapSchema)sc).ValueSchema, seen), Symbol.String), + Symbol.MapStart); + case Schema.Type.Record: + { + LitS wsc = new LitS(sc); + Symbol rresult = seen.ContainsKey(wsc) ? seen[wsc] : null; + if (rresult == null) + { + Symbol[] production = new Symbol[((RecordSchema)sc).Fields.Count]; + + // We construct a symbol without filling the array. Please see + // <seealso cref="Symbol.production"/> for the reason. + rresult = Symbol.NewSeq(production); + seen[wsc] = rresult; + + int j = production.Length; + foreach (Field f in ((RecordSchema)sc).Fields) + { + production[--j] = Generate(f.Schema, seen); + } + } + + return rresult; + } + case Schema.Type.Union: + IList<Schema> subs = ((UnionSchema)sc).Schemas; + Symbol[] symbols = new Symbol[subs.Count]; + string[] labels = new string[subs.Count]; + + int i = 0; + foreach (Schema b in ((UnionSchema)sc).Schemas) + { + symbols[i] = Generate(b, seen); + labels[i] = b.Fullname; + i++; + } + + return Symbol.NewSeq(Symbol.NewAlt(symbols, labels), Symbol.Union); + + default: + throw new Exception("Unexpected schema type"); + } + } + + /// <summary> + /// A wrapper around Schema that does "==" equality. </summary> + protected class LitS + { + private readonly Schema actual; + + public LitS(Schema actual) + { + this.actual = actual; + } + + /// <summary> + /// Two LitS are equal if and only if their underlying schema is the same (not + /// merely equal). + /// </summary> + public override bool Equals(object o) + { + if (!(o is LitS)) Review Comment: ## Equals should not apply "is" LitS.Equals(object) should not use "is" on its parameter, as it will not work properly for subclasses of LitS. [Show more details](https://github.com/apache/avro/security/code-scanning/2902) ########## lang/csharp/src/apache/main/IO/Parsing/Symbol.cs: ########## @@ -0,0 +1,778 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; + +namespace Avro.IO.Parsing +{ + /// <summary> + /// Symbol is the base of all symbols (terminals and non-terminals) of the + /// grammar. + /// </summary> + public abstract class Symbol + { + /// <summary> + /// The type of symbol. + /// </summary> + public enum Kind + { + /// <summary> + /// terminal symbols which have no productions </summary> + Terminal, + + /// <summary> + /// Start symbol for some grammar </summary> + Root, + + /// <summary> + /// non-terminal symbol which is a sequence of one or more other symbols </summary> + Sequence, + + /// <summary> + /// non-terminal to represent the contents of an array or map </summary> + Repeater, + + /// <summary> + /// non-terminal to represent the union </summary> + Alternative, + + /// <summary> + /// non-terminal action symbol which are automatically consumed </summary> + ImplicitAction, + + /// <summary> + /// non-terminal action symbol which is explicitly consumed </summary> + ExplicitAction + } + + /// The kind of this symbol. + public readonly Kind SymKind; + + /// <summary> + /// The production for this symbol. If this symbol is a terminal this is + /// <tt>null</tt>. Otherwise this holds the the sequence of the symbols that + /// forms the production for this symbol. The sequence is in the reverse order of + /// production. This is useful for easy copying onto parsing stack. + /// + /// Please note that this is a final. So the production for a symbol should be + /// known before that symbol is constructed. This requirement cannot be met for + /// those symbols which are recursive (e.g. a record that holds union a branch of + /// which is the record itself). To resolve this problem, we initialize the + /// symbol with an array of nulls. Later we fill the symbols. Not clean, but + /// works. The other option is to not have this field a final. But keeping it + /// final and thus keeping symbol immutable gives some comfort. See various + /// generators how we generate records. + /// </summary> + public readonly Symbol[] Production; + + /// <summary> + /// Constructs a new symbol of the given kind. + /// </summary> + protected Symbol(Kind kind) : this(kind, null) + { + } + + /// <summary> + /// Constructs a new symbol of the given kind and production. + /// </summary> + protected Symbol(Kind kind, Symbol[] production) + { + this.Production = production; + this.SymKind = kind; + } + + /// <summary> + /// A convenience method to construct a root symbol. + /// </summary> + public static Symbol NewRoot(params Symbol[] symbols) + { + return new Root(symbols); + } + + /// <summary> + /// A convenience method to construct a sequence. + /// </summary> + /// <param name="production"> The constituent symbols of the sequence. </param> + public static Symbol NewSeq(params Symbol[] production) + { + return new Sequence(production); + } + + /// <summary> + /// A convenience method to construct a repeater. + /// </summary> + /// <param name="endSymbol"> The end symbol. </param> + /// <param name="symsToRepeat"> The symbols to repeat in the repeater. </param> + public static Symbol NewRepeat(Symbol endSymbol, params Symbol[] symsToRepeat) + { + return new Repeater(endSymbol, symsToRepeat); + } + + /// <summary> + /// A convenience method to construct a union. + /// </summary> + public static Symbol NewAlt(Symbol[] symbols, string[] labels) + { + return new Alternative(symbols, labels); + } + + /// <summary> + /// A convenience method to construct an ErrorAction. + /// </summary> + /// <param name="e"> </param> + protected static Symbol Error(string e) + { + return new ErrorAction(e); + } + + /// <summary> + /// A convenience method to construct a ResolvingAction. + /// </summary> + /// <param name="w"> The writer symbol </param> + /// <param name="r"> The reader symbol </param> + protected static Symbol Resolve(Symbol w, Symbol r) + { + return new ResolvingAction(w, r); + } + + protected class Fixup + { + public readonly Symbol[] Symbols; + public readonly int Pos; + + public Fixup(Symbol[] symbols, int pos) + { + this.Symbols = symbols; + this.Pos = pos; + } + } + + protected virtual Symbol Flatten(IDictionary<Sequence, Sequence> map, IDictionary<Sequence, IList<Fixup>> map2) + { + return this; + } + + public virtual int FlattenedSize() + { + return 1; + } + + /// <summary> + /// Flattens the given sub-array of symbols into an sub-array of symbols. Every + /// <tt>Sequence</tt> in the input are replaced by its production recursively. + /// Non-<tt>Sequence</tt> symbols, they internally have other symbols those + /// internal symbols also get flattened. When flattening is done, the only place + /// there might be Sequence symbols is in the productions of a Repeater, + /// Alternative, or the symToParse and symToSkip in a UnionAdjustAction or + /// SkipAction. + /// + /// Why is this done? We want our parsers to be fast. If we left the grammars + /// unflattened, then the parser would be constantly copying the contents of + /// nested Sequence productions onto the parsing stack. Instead, because of + /// flattening, we have a long top-level production with no Sequences unless the + /// Sequence is absolutely needed, e.g., in the case of a Repeater or an + /// Alternative. + /// + /// Well, this is not exactly true when recursion is involved. Where there is a + /// recursive record, that record will be "inlined" once, but any internal (ie, + /// recursive) references to that record will be a Sequence for the record. That + /// Sequence will not further inline itself -- it will refer to itself as a + /// Sequence. The same is true for any records nested in this outer recursive + /// record. Recursion is rare, and we want things to be fast in the typical case, + /// which is why we do the flattening optimization. + /// + /// + /// The algorithm does a few tricks to handle recursive symbol definitions. In + /// order to avoid infinite recursion with recursive symbols, we have a map of + /// Symbol->Symbol. Before fully constructing a flattened symbol for a + /// <tt>Sequence</tt> we insert an empty output symbol into the map and then + /// start filling the production for the <tt>Sequence</tt>. If the same + /// <tt>Sequence</tt> is encountered due to recursion, we simply return the + /// (empty) output <tt>Sequence</tt> from the map. Then we actually fill out + /// the production for the <tt>Sequence</tt>. As part of the flattening process + /// we copy the production of <tt>Sequence</tt>s into larger arrays. If the + /// original <tt>Sequence</tt> has not not be fully constructed yet, we copy a + /// bunch of <tt>null</tt>s. Fix-up remembers all those <tt>null</tt> patches. + /// The fix-ups gets finally filled when we know the symbols to occupy those + /// patches. + /// </summary> + /// <param name="in"> The array of input symbols to flatten </param> + /// <param name="start"> The position where the input sub-array starts. </param> + /// <param name="out"> The output that receives the flattened list of symbols. The + /// output array should have sufficient space to receive the + /// expanded sub-array of symbols. </param> + /// <param name="skip"> The position where the output input sub-array starts. </param> + /// <param name="map"> A map of symbols which have already been expanded. Useful for + /// handling recursive definitions and for caching. </param> + /// <param name="map2"> A map to to store the list of fix-ups. </param> + protected static void Flatten(Symbol[] @in, int start, Symbol[] @out, int skip, + IDictionary<Sequence, Sequence> map, IDictionary<Sequence, IList<Fixup>> map2) + { + for (int i = start, j = skip; i < @in.Length; i++) + { + Symbol s = @in[i].Flatten(map, map2); + if (s is Sequence) + { + Symbol[] p = s.Production; + IList<Fixup> l = map2.ContainsKey((Sequence)s) ? map2[(Sequence)s] : null; Review Comment: ## Inefficient use of ContainsKey Inefficient use of 'ContainsKey' and [indexer](1). [Show more details](https://github.com/apache/avro/security/code-scanning/2899) ########## lang/csharp/src/apache/main/IO/Parsing/ValidatingGrammarGenerator.cs: ########## @@ -0,0 +1,152 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; +using System.Collections.Generic; + +namespace Avro.IO.Parsing +{ + /// <summary> + /// The class that generates validating grammar. + /// </summary> + public class ValidatingGrammarGenerator + { + /// <summary> + /// Returns the non-terminal that is the start symbol for the grammar for the + /// given schema <tt>sc</tt>. + /// </summary> + public virtual Symbol Generate(Schema schema) + { + return Symbol.NewRoot(Generate(schema, new Dictionary<LitS, Symbol>())); + } + + /// <summary> + /// Returns the non-terminal that is the start symbol for the grammar for the + /// given schema <tt>sc</tt>. If there is already an entry for the given schema + /// in the given map <tt>seen</tt> then that entry is returned. Otherwise a new + /// symbol is generated and an entry is inserted into the map. + /// </summary> + /// <param name="sc"> The schema for which the start symbol is required </param> + /// <param name="seen"> A map of schema to symbol mapping done so far. </param> + /// <returns> The start symbol for the schema </returns> + protected virtual Symbol Generate(Schema sc, IDictionary<LitS, Symbol> seen) + { + switch (sc.Tag) + { + case Schema.Type.Null: + return Symbol.Null; + case Schema.Type.Boolean: + return Symbol.Boolean; + case Schema.Type.Int: + return Symbol.Int; + case Schema.Type.Long: + return Symbol.Long; + case Schema.Type.Float: + return Symbol.Float; + case Schema.Type.Double: + return Symbol.Double; + case Schema.Type.String: + return Symbol.String; + case Schema.Type.Bytes: + return Symbol.Bytes; + case Schema.Type.Fixed: + return Symbol.NewSeq(new Symbol.IntCheckAction(((FixedSchema)sc).Size), Symbol.Fixed); + case Schema.Type.Enumeration: + return Symbol.NewSeq(new Symbol.IntCheckAction(((EnumSchema)sc).Symbols.Count), Symbol.Enum); + case Schema.Type.Array: + return Symbol.NewSeq( + Symbol.NewRepeat(Symbol.ArrayEnd, Generate(((ArraySchema)sc).ItemSchema, seen)), + Symbol.ArrayStart); + case Schema.Type.Map: + return Symbol.NewSeq( + Symbol.NewRepeat(Symbol.MapEnd, Generate(((MapSchema)sc).ValueSchema, seen), Symbol.String), + Symbol.MapStart); + case Schema.Type.Record: + { + LitS wsc = new LitS(sc); + Symbol rresult = seen.ContainsKey(wsc) ? seen[wsc] : null; Review Comment: ## Inefficient use of ContainsKey Inefficient use of 'ContainsKey' and [indexer](1). [Show more details](https://github.com/apache/avro/security/code-scanning/2901) ########## lang/csharp/src/apache/main/IO/Parsing/JsonGrammarGenerator.cs: ########## @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; +using System.Collections.Generic; + +namespace Avro.IO.Parsing +{ + /// <summary> + /// The class that generates a grammar suitable to parse Avro data in JSON + /// format. + /// </summary> + public class JsonGrammarGenerator : ValidatingGrammarGenerator + { + /// <summary> + /// Returns the non-terminal that is the start symbol for the grammar for the + /// grammar for the given schema <tt>sc</tt>. + /// </summary> + public override Symbol Generate(Schema schema) + { + return Symbol.NewRoot(Generate(schema, new Dictionary<LitS, Symbol>())); + } + + /// <summary> + /// Returns the non-terminal that is the start symbol for grammar of the given + /// schema <tt>sc</tt>. If there is already an entry for the given schema in the + /// given map <tt>seen</tt> then that entry is returned. Otherwise a new symbol + /// is generated and an entry is inserted into the map. + /// </summary> + /// <param name="sc"> The schema for which the start symbol is required </param> + /// <param name="seen"> A map of schema to symbol mapping done so far. </param> + /// <returns> The start symbol for the schema </returns> + protected override Symbol Generate(Schema sc, IDictionary<LitS, Symbol> seen) + { + switch (sc.Tag) + { + case Schema.Type.Null: + case Schema.Type.Boolean: + case Schema.Type.Int: + case Schema.Type.Long: + case Schema.Type.Float: + case Schema.Type.Double: + case Schema.Type.String: + case Schema.Type.Bytes: + case Schema.Type.Fixed: + case Schema.Type.Union: + return base.Generate(sc, seen); + case Schema.Type.Enumeration: + return Symbol.NewSeq(new Symbol.EnumLabelsAction(((EnumSchema)sc).Symbols), Symbol.Enum); + case Schema.Type.Array: + return Symbol.NewSeq( + Symbol.NewRepeat(Symbol.ArrayEnd, Symbol.ItemEnd, Generate(((ArraySchema)sc).ItemSchema, seen)), + Symbol.ArrayStart); + case Schema.Type.Map: + return Symbol.NewSeq( + Symbol.NewRepeat(Symbol.MapEnd, Symbol.ItemEnd, Generate(((MapSchema)sc).ValueSchema, seen), + Symbol.MapKeyMarker, Symbol.String), Symbol.MapStart); + case Schema.Type.Record: + { + LitS wsc = new LitS(sc); + Symbol rresult = seen.ContainsKey(wsc) ? seen[wsc] : null; + if (rresult == null) + { + Symbol[] production = new Symbol[((RecordSchema)sc).Fields.Count * 3 + 2]; + rresult = Symbol.NewSeq(production); + seen[wsc] = rresult; + + int i = production.Length; + int n = 0; + production[--i] = Symbol.RecordStart; + foreach (Field f in ((RecordSchema)sc).Fields) + { + production[--i] = Symbol.fieldAdjustAction(n, f.Name, f.Aliases); + production[--i] = Generate(f.Schema, seen); + production[--i] = Symbol.FieldEnd; + n++; + } + + production[--i] = Symbol.RecordEnd; Review Comment: ## Useless assignment to local variable This assignment to [i](1) is useless, since its value is never read. [Show more details](https://github.com/apache/avro/security/code-scanning/2896) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
