From 963c3c11fbd5ccb6d77f084a315fe66d4bc7d2bf Mon Sep 17 00:00:00 2001 From: TSUYUSATO Kitsune Date: Sat, 10 Nov 2018 00:15:26 +0900 Subject: [PATCH 1/2] Use third-person singular It is my failure since #7029. Sorry... --- spec/compiler/crystal/tools/doc/macro_spec.cr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/compiler/crystal/tools/doc/macro_spec.cr b/spec/compiler/crystal/tools/doc/macro_spec.cr index 8a3b7f0a3ad6..70f15ea2100e 100644 --- a/spec/compiler/crystal/tools/doc/macro_spec.cr +++ b/spec/compiler/crystal/tools/doc/macro_spec.cr @@ -52,7 +52,7 @@ describe Doc::Macro do doc_macro.args_to_s.should eq("(**foo)") end - it "show simple arg and double splat arg" do + it "shows simple arg and double splat arg" do program = Program.new generator = Doc::Generator.new program, ["."], ".", nil doc_type = Doc::Type.new generator, program @@ -62,7 +62,7 @@ describe Doc::Macro do doc_macro.args_to_s.should eq("(foo, **bar)") end - it "show block arg" do + it "shows block arg" do program = Program.new generator = Doc::Generator.new program, ["."], ".", nil doc_type = Doc::Type.new generator, program @@ -72,7 +72,7 @@ describe Doc::Macro do doc_macro.args_to_s.should eq("(&foo)") end - it "show simple arg and block arg" do + it "shows simple arg and block arg" do program = Program.new generator = Doc::Generator.new program, ["."], ".", nil doc_type = Doc::Type.new generator, program From fee70f1e35d465096bb49f2610b737ee68f77399 Mon Sep 17 00:00:00 2001 From: TSUYUSATO Kitsune Date: Sat, 10 Nov 2018 01:12:50 +0900 Subject: [PATCH 2/2] Doc: prevent XSS via args An external name of method arg is not HTML-escaped (and missing quotes even if needed). We can do XSS by this easily. Otherwise, `typeof` restriction is not HTML-escaped, we can also do XSS by this. --- spec/compiler/crystal/tools/doc/macro_spec.cr | 30 ++++++++++++++ .../compiler/crystal/tools/doc/method_spec.cr | 40 +++++++++++++++++++ spec/support/syntax.cr | 4 +- src/compiler/crystal/tools/doc/macro.cr | 27 +++++++++++-- src/compiler/crystal/tools/doc/method.cr | 8 +++- src/compiler/crystal/tools/doc/type.cr | 2 +- 6 files changed, 104 insertions(+), 7 deletions(-) diff --git a/spec/compiler/crystal/tools/doc/macro_spec.cr b/spec/compiler/crystal/tools/doc/macro_spec.cr index 70f15ea2100e..5d3a0ba65942 100644 --- a/spec/compiler/crystal/tools/doc/macro_spec.cr +++ b/spec/compiler/crystal/tools/doc/macro_spec.cr @@ -81,5 +81,35 @@ describe Doc::Macro do doc_macro = Doc::Macro.new generator, doc_type, a_macro doc_macro.args_to_s.should eq("(foo, &bar)") end + + it "shows external name of arg" do + program = Program.new + generator = Doc::Generator.new program, ["."], ".", nil + doc_type = Doc::Type.new generator, program + + a_macro = Macro.new "foo", ["foo".arg(external_name: "bar")] + doc_macro = Doc::Macro.new generator, doc_type, a_macro + doc_macro.args_to_s.should eq("(bar foo)") + end + + it "shows external name of arg with quotes and escaping" do + program = Program.new + generator = Doc::Generator.new program, ["."], ".", nil + doc_type = Doc::Type.new generator, program + + a_macro = Macro.new "foo", ["foo".arg(external_name: "<<-< uouo fish life")] + doc_macro = Doc::Macro.new generator, doc_type, a_macro + doc_macro.args_to_s.should eq("("<<-< uouo fish life" foo)") + end + + it "shows default value with highlighting" do + program = Program.new + generator = Doc::Generator.new program, ["."], ".", nil + doc_type = Doc::Type.new generator, program + + a_macro = Macro.new "foo", ["foo".arg(default_value: 1.int32)] + doc_macro = Doc::Macro.new generator, doc_type, a_macro + doc_macro.args_to_s.should eq(%((foo = 1))) + end end end diff --git a/spec/compiler/crystal/tools/doc/method_spec.cr b/spec/compiler/crystal/tools/doc/method_spec.cr index 4934026c9cda..f98467a7cd47 100644 --- a/spec/compiler/crystal/tools/doc/method_spec.cr +++ b/spec/compiler/crystal/tools/doc/method_spec.cr @@ -71,5 +71,45 @@ describe Doc::Method do doc_method = Doc::Method.new generator, doc_type, a_def, false doc_method.args_to_s.should eq("(foo) : Foo") end + + it "shows external name of arg" do + program = Program.new + generator = Doc::Generator.new program, ["."], ".", nil + doc_type = Doc::Type.new generator, program + + a_def = Def.new "foo", ["foo".arg(external_name: "bar")] + doc_method = Doc::Method.new generator, doc_type, a_def, false + doc_method.args_to_s.should eq("(bar foo)") + end + + it "shows external name of arg with quotes and escaping" do + program = Program.new + generator = Doc::Generator.new program, ["."], ".", nil + doc_type = Doc::Type.new generator, program + + a_def = Def.new "foo", ["foo".arg(external_name: "<<-< uouo fish life")] + doc_method = Doc::Method.new generator, doc_type, a_def, false + doc_method.args_to_s.should eq("("<<-< uouo fish life" foo)") + end + + it "shows typeof restriction of arg with highlighting" do + program = Program.new + generator = Doc::Generator.new program, ["."], ".", nil + doc_type = Doc::Type.new generator, program + + a_def = Def.new "foo", ["foo".arg(restriction: TypeOf.new([1.int32] of ASTNode))] + doc_method = Doc::Method.new generator, doc_type, a_def, false + doc_method.args_to_s.should eq(%((foo : typeof(1)))) + end + + it "shows default value of arg with highlighting" do + program = Program.new + generator = Doc::Generator.new program, ["."], ".", nil + doc_type = Doc::Type.new generator, program + + a_def = Def.new "foo", ["foo".arg(default_value: 1.int32)] + doc_method = Doc::Method.new generator, doc_type, a_def, false + doc_method.args_to_s.should eq(%((foo = 1))) + end end end diff --git a/spec/support/syntax.cr b/spec/support/syntax.cr index 08f84d082018..8d036541256a 100644 --- a/spec/support/syntax.cr +++ b/spec/support/syntax.cr @@ -54,8 +54,8 @@ class String Var.new self end - def arg(default_value = nil, restriction = nil) - Arg.new self, default_value: default_value, restriction: restriction + def arg(default_value = nil, restriction = nil, external_name = nil) + Arg.new self, default_value: default_value, restriction: restriction, external_name: external_name end def call diff --git a/src/compiler/crystal/tools/doc/macro.cr b/src/compiler/crystal/tools/doc/macro.cr index 3d83040ddd91..be47005cc121 100644 --- a/src/compiler/crystal/tools/doc/macro.cr +++ b/src/compiler/crystal/tools/doc/macro.cr @@ -72,26 +72,47 @@ class Crystal::Doc::Macro @macro.args.each_with_index do |arg, i| io << ", " if printed io << '*' if @macro.splat_index == i - io << arg + arg_to_s arg, io printed = true end if double_splat = @macro.double_splat io << ", " if printed io << "**" - io << double_splat + arg_to_s double_splat, io printed = true end if block_arg = @macro.block_arg io << ", " if printed io << '&' - io << block_arg + arg_to_s block_arg, io end io << ')' end + def arg_to_s(arg : Arg, io) + if arg.external_name != arg.name + name = arg.external_name.empty? ? "_" : arg.external_name + if Symbol.needs_quotes? name + HTML.escape name.inspect, io + else + io << name + end + io << ' ' + end + + io << arg.name + + # Macro arg cannot not have a restriction. + + if default_value = arg.default_value + io << " = " + io << Highlighter.highlight(default_value.to_s) + end + end + def has_args? !@macro.args.empty? || @macro.double_splat || @macro.block_arg end diff --git a/src/compiler/crystal/tools/doc/method.cr b/src/compiler/crystal/tools/doc/method.cr index c01fd2c01db2..6ca92a3cc547 100644 --- a/src/compiler/crystal/tools/doc/method.cr +++ b/src/compiler/crystal/tools/doc/method.cr @@ -172,7 +172,12 @@ class Crystal::Doc::Method def arg_to_html(arg : Arg, io, links = true) if arg.external_name != arg.name - io << (arg.external_name.empty? ? '_' : arg.external_name) + name = arg.external_name.empty? ? "_" : arg.external_name + if Symbol.needs_quotes? name + HTML.escape name.inspect, io + else + io << name + end io << ' ' end @@ -185,6 +190,7 @@ class Crystal::Doc::Method io << " : " @type.type_to_html type, io, links: links end + if default_value = arg.default_value io << " = " io << Highlighter.highlight(default_value.to_s) diff --git a/src/compiler/crystal/tools/doc/type.cr b/src/compiler/crystal/tools/doc/type.cr index 88412c5182e6..bff6258553f0 100644 --- a/src/compiler/crystal/tools/doc/type.cr +++ b/src/compiler/crystal/tools/doc/type.cr @@ -576,7 +576,7 @@ class Crystal::Doc::Type end def node_to_html(node, io, links = true) - io << node + io << Highlighter.highlight(node.to_s) end def type_to_html(type)