Repository: anz-bank/sysl
Pull request #665 (2020-03-03 03:03:00)

Add escaping features in unhandled endpoints
Changes proposed in this pull request:
- Add escaping features in unhandled endpoints in call statements and collector statements (related to #658 )

Checklist:
- [x] Added related tests
- [x] Made corresponding changes to the documentation

@anz-bank/sysl-developers


joshcarp: Nice

Patch:
diff --git a/demo/examples/Special-Characters/1_project.sysl b/demo/examples/Special-Characters/1_project.sysl
index 4cf79dc7..d5eb07ad 100644
--- a/demo/examples/Special-Characters/1_project.sysl
+++ b/demo/examples/Special-Characters/1_project.sysl
@@ -11,4 +11,5 @@ testapp "Simple" [package="package_foo"]:
     # In this case, `%25` will be converted to `%`
     /tests%25again:
         GET:
+            . <- GET /test%2epdf
             return set of SimpleObj
diff --git a/docs/website/content/docs/byexample/special-characters.md b/docs/website/content/docs/byexample/special-characters.md
index 989e4d0c..d2cd651d 100644
--- a/docs/website/content/docs/byexample/special-characters.md
+++ b/docs/website/content/docs/byexample/special-characters.md
@@ -6,7 +6,7 @@ description= ""
 layout= "byexample"
 weight = 12
 topic = "Web protocols"
-PlaygroundURL = "http://anz-bank.github.io/sysl-playground/?input=dGVzdGFwcCAiU2ltcGxlIiBbcGFja2FnZT0icGFja2FnZV9mb28iXToKICAgIC90ZXN0JTJlcGRmOgogICAgICAgIEdFVDoKICAgICAgICAgICAgcmV0dXJuIDIwMCA8OiBTaW1wbGVPYmoKICAgICAgICBERUxFVEU6CiAgICAgICAgICAgIHJldHVybiAyMDMKICAgIC90ZXN0cyUyNWFnYWluOgogICAgICAgIEdFVDoKICAgICAgICAgICAgcmV0dXJuIHNldCBvZiBTaW1wbGVPYmoK&cmd="
+PlaygroundURL = "http://anz-bank.github.io/sysl-playground/?input=dGVzdGFwcCAiU2ltcGxlIiBbcGFja2FnZT0icGFja2FnZV9mb28iXToKICAgIC90ZXN0JTJlcGRmOgogICAgICAgIEdFVDoKICAgICAgICAgICAgcmV0dXJuIDIwMCA8OiBTaW1wbGVPYmoKICAgICAgICBERUxFVEU6CiAgICAgICAgICAgIHJldHVybiAyMDMKICAgIC90ZXN0cyUyNWFnYWluOgogICAgICAgIEdFVDoKICAgICAgICAgICAgLiA8LSBHRVQgL3Rlc3QlMmVwZGYKICAgICAgICAgICAgcmV0dXJuIHNldCBvZiBTaW1wbGVPYmoK&cmd="
 ID = "special-characters"
 CodeWithoutComments = """testapp "Simple" [package="package_foo"]:
     /test%2epdf:
@@ -16,6 +16,7 @@ CodeWithoutComments = """testapp "Simple" [package="package_foo"]:
             return 203
     /tests%25again:
         GET:
+            . <- GET /test%2epdf
             return set of SimpleObj
 """
 
@@ -34,6 +35,7 @@ Segs = [[
       {CodeEmpty= false,CodeLeading= false,CodeRun= false,CodeRendered="""<pre class="chroma">
     <span class="o">/</span><span class="nx">tests</span><span class="o">%</span><span class="mi">25</span><span class="nx">again</span><span class="p">:</span>
         <span class="nx">GET</span><span class="p">:</span>
+            <span class="p">.</span> <span class="o">&lt;-</span> <span class="nx">GET</span> <span class="o">/</span><span class="nx">test</span><span class="o">%</span><span class="mi">2</span><span class="nx">epdf</span>
             <span class="k">return</span> <span class="nx">set</span> <span class="nx">of</span> <span class="nx">SimpleObj</span></pre>""",DocsRendered= """<p>In this case, <code>%25</code> will be converted to <code>%</code></p>
 """,Image = ""},
 
diff --git a/pkg/importer/utils.go b/pkg/importer/utils.go
index 589e593d..960395b5 100644
--- a/pkg/importer/utils.go
+++ b/pkg/importer/utils.go
@@ -74,14 +74,17 @@ func spaceSeparate(items ...string) string {
 func getSyslSafeEndpoint(endpoint string) string {
 	// url.PathEscape does not escape '.'
 	charsToKeep := map[string]string{
-		"2F": "/",
-		"7B": "{",
-		"7D": "}",
+		`%2F`: "/",
+		`%7B`: "{",
+		`%7D`: "}",
+		`%3D`: "=",
+		`%3F`: "?",
+		`%26`: "&",
 	}
 	endpoint = url.PathEscape(endpoint)
-	endpoint = strings.ReplaceAll(endpoint, ".", "%"+"2E")
+	endpoint = strings.ReplaceAll(endpoint, ".", `%2E`)
 	for hex, realChar := range charsToKeep {
-		endpoint = strings.ReplaceAll(endpoint, "%"+hex, realChar)
+		endpoint = strings.ReplaceAll(endpoint, hex, realChar)
 	}
 	return endpoint
 }
diff --git a/pkg/parse/listener_impl.go b/pkg/parse/listener_impl.go
index be7ab0a7..26068a8a 100644
--- a/pkg/parse/listener_impl.go
+++ b/pkg/parse/listener_impl.go
@@ -4,7 +4,6 @@ package parse // SyslParser
 import (
 	"encoding/json"
 	"fmt"
-	"net/url"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -997,11 +996,7 @@ func (s *TreeShapeListener) EnterHttp_path(ctx *parser.Http_pathContext) {
 func (s *TreeShapeListener) ExitHttp_path(*parser.Http_pathContext) {
 	// s.endpointName is built along as we enter http_path/http_path_suffix/http_path_var_with_type
 	// commit this value to urlPrefixes
-	endpoint, err := url.PathUnescape(s.endpointName)
-	if err != nil {
-		panic(err)
-	}
-	s.urlPrefixes.Push(endpoint)
+	s.urlPrefixes.Push(MustUnescape(s.endpointName))
 }
 
 // EnterRet_stmt is called when production ret_stmt is entered.
@@ -1044,7 +1039,7 @@ func (s *TreeShapeListener) EnterCall_stmt(ctx *parser.Call_stmtContext) {
 		Stmt: &sysl.Statement_Call{
 			Call: &sysl.Call{
 				Target:   appName,
-				Endpoint: ctx.Target_endpoint().GetText(),
+				Endpoint: MustUnescape(ctx.Target_endpoint().GetText()),
 			},
 		},
 	})
@@ -1688,7 +1683,7 @@ func (s *TreeShapeListener) EnterCollector_call_stmt(ctx *parser.Collector_call_
 		Stmt: &sysl.Statement_Call{
 			Call: &sysl.Call{
 				Target:   appName,
-				Endpoint: strings.TrimSpace(ctx.Target_endpoint().GetText()),
+				Endpoint: MustUnescape(ctx.Target_endpoint().GetText()),
 			},
 		},
 	})
@@ -1696,7 +1691,8 @@ func (s *TreeShapeListener) EnterCollector_call_stmt(ctx *parser.Collector_call_
 
 // EnterCollector_http_stmt is called when production collector_http_stmt is entered.
 func (s *TreeShapeListener) EnterCollector_http_stmt(ctx *parser.Collector_http_stmtContext) {
-	text := strings.TrimSpace(ctx.HTTP_VERBS().GetText()) + " " + ctx.Collector_http_stmt_suffix().GetText()
+	text := strings.TrimSpace(ctx.HTTP_VERBS().GetText()) + " " +
+		MustUnescape(ctx.Collector_http_stmt_suffix().GetText())
 
 	s.addToCurrentScope(&sysl.Statement{
 		Stmt: &sysl.Statement_Action{
diff --git a/pkg/parse/utils.go b/pkg/parse/utils.go
index e0603baa..91760d06 100644
--- a/pkg/parse/utils.go
+++ b/pkg/parse/utils.go
@@ -2,6 +2,7 @@ package parse
 
 import (
 	"math"
+	"net/url"
 	"strings"
 
 	"github.com/antlr/antlr4/runtime/Go/antlr"
@@ -145,3 +146,12 @@ func (p *PathStack) update() string {
 	p.path = p.prefix + strings.Join(p.parts, p.sep)
 	return p.path
 }
+
+// MustUnescape will escape URL escaped characters.
+func MustUnescape(endpoint string) string {
+	s, err := url.PathUnescape(endpoint)
+	if err != nil {
+		panic(err)
+	}
+	return strings.TrimSpace(s)
+}


Test patch:
diff --git a/pkg/parse/Parser_test.go b/pkg/parse/Parser_test.go
index d052d3ac..6b08e3ab 100644
--- a/pkg/parse/Parser_test.go
+++ b/pkg/parse/Parser_test.go
@@ -136,7 +136,6 @@ func parseComparable(
 			}
 		}
 	}
-
 	return module, nil
 }
 
@@ -567,6 +566,12 @@ func TestAlias(t *testing.T) {
 	testParseAgainstGolden(t, "tests/alias_inline.sysl", "")
 }
 
+func TestEscapedEndpoints(t *testing.T) {
+	t.Parallel()
+
+	testParseAgainstGolden(t, "tests/endpoints.sysl", "")
+}
+
 func TestStrings(t *testing.T) {
 	t.Parallel()
 
diff --git a/pkg/parse/tests/endpoints.sysl b/pkg/parse/tests/endpoints.sysl
new file mode 100644
index 00000000..51cbba31
--- /dev/null
+++ b/pkg/parse/tests/endpoints.sysl
@@ -0,0 +1,27 @@
+My :: Server:
+    /serve%2ehtml:
+        GET:
+            return ok <: App.Response
+
+    /more/stuff%2eendpoint/{id <: string}:
+        POST:
+            return ok <: App.Response
+
+App:
+    /more/stuff%2eendpoint/{id <: string}:
+        POST:
+            . <- GET /serve%2ehtml
+            return ok <: Response
+
+    /serve%2ehtml:
+        GET:
+            My :: Server <- GET /serve%2ehtml [~https]
+            return ok <: Response
+
+    !type Response:
+        response <: string
+App:
+    .. * <- *:
+        POST /more/stuff%2eendpoint/{id} [id = "one"]
+        App <- GET /serve%2ehtml [id="doesn't matter"]
+        My :: Server <- GET /serve%2ehtml [id="doesn't matter"]
diff --git a/pkg/parse/tests/endpoints.sysl.golden.textpb b/pkg/parse/tests/endpoints.sysl.golden.textpb
new file mode 100644
index 00000000..e3d7777b
--- /dev/null
+++ b/pkg/parse/tests/endpoints.sysl.golden.textpb
@@ -0,0 +1,266 @@
+apps: <
+  key: "App"
+  value: <
+    name: <
+      part: "App"
+    >
+    endpoints: <
+      key: ".. * <- *"
+      value: <
+        name: ".. * <- *"
+        stmt: <
+          action: <
+            action: "POST /more/stuff.endpoint/{id}"
+          >
+          attrs: <
+            key: "id"
+            value: <
+              s: "one"
+            >
+          >
+        >
+        stmt: <
+          call: <
+            target: <
+              part: "App"
+            >
+            endpoint: "GET /serve.html"
+          >
+          attrs: <
+            key: "id"
+            value: <
+              s: "doesn't matter"
+            >
+          >
+        >
+        stmt: <
+          call: <
+            target: <
+              part: "My"
+              part: "Server"
+            >
+            endpoint: "GET /serve.html"
+          >
+          attrs: <
+            key: "id"
+            value: <
+              s: "doesn't matter"
+            >
+          >
+        >
+      >
+    >
+    endpoints: <
+      key: "GET /serve.html"
+      value: <
+        name: "GET /serve.html"
+        attrs: <
+          key: "patterns"
+          value: <
+            a: <
+              elt: <
+                s: "rest"
+              >
+            >
+          >
+        >
+        stmt: <
+          call: <
+            target: <
+              part: "My"
+              part: "Server"
+            >
+            endpoint: "GET /serve.html"
+          >
+          attrs: <
+            key: "id"
+            value: <
+              s: "doesn't matter"
+            >
+          >
+          attrs: <
+            key: "patterns"
+            value: <
+              a: <
+                elt: <
+                  s: "https"
+                >
+              >
+            >
+          >
+        >
+        stmt: <
+          ret: <
+            payload: "ok <: Response"
+          >
+        >
+        rest_params: <
+          method: GET
+          path: "/serve.html"
+        >
+      >
+    >
+    endpoints: <
+      key: "POST /more/stuff.endpoint/{id}"
+      value: <
+        name: "POST /more/stuff.endpoint/{id}"
+        attrs: <
+          key: "id"
+          value: <
+            s: "one"
+          >
+        >
+        attrs: <
+          key: "patterns"
+          value: <
+            a: <
+              elt: <
+                s: "rest"
+              >
+            >
+          >
+        >
+        stmt: <
+          call: <
+            target: <
+              part: "App"
+            >
+            endpoint: "GET /serve.html"
+          >
+          attrs: <
+            key: "id"
+            value: <
+              s: "doesn't matter"
+            >
+          >
+        >
+        stmt: <
+          ret: <
+            payload: "ok <: Response"
+          >
+        >
+        rest_params: <
+          method: POST
+          path: "/more/stuff.endpoint/{id}"
+          url_param: <
+            name: "id"
+            type: <
+              primitive: STRING
+              source_context: <
+                file: "tests/endpoints.sysl"
+                start: <
+                  line: 11
+                  col: 27
+                >
+                end: <
+                  line: 11
+                  col: 40
+                >
+              >
+            >
+          >
+        >
+      >
+    >
+    types: <
+      key: "Response"
+      value: <
+        tuple: <
+          attr_defs: <
+            key: "response"
+            value: <
+              primitive: STRING
+              source_context: <
+                file: "tests/endpoints.sysl"
+                start: <
+                  line: 22
+                  col: 20
+                >
+                end: <
+                  line: 22
+                  col: 20
+                >
+              >
+            >
+          >
+        >
+      >
+    >
+  >
+>
+apps: <
+  key: "My :: Server"
+  value: <
+    name: <
+      part: "My"
+      part: "Server"
+    >
+    endpoints: <
+      key: "GET /serve.html"
+      value: <
+        name: "GET /serve.html"
+        attrs: <
+          key: "patterns"
+          value: <
+            a: <
+              elt: <
+                s: "rest"
+              >
+            >
+          >
+        >
+        stmt: <
+          ret: <
+            payload: "ok <: App.Response"
+          >
+        >
+        rest_params: <
+          method: GET
+          path: "/serve.html"
+        >
+      >
+    >
+    endpoints: <
+      key: "POST /more/stuff.endpoint/{id}"
+      value: <
+        name: "POST /more/stuff.endpoint/{id}"
+        attrs: <
+          key: "patterns"
+          value: <
+            a: <
+              elt: <
+                s: "rest"
+              >
+            >
+          >
+        >
+        stmt: <
+          ret: <
+            payload: "ok <: App.Response"
+          >
+        >
+        rest_params: <
+          method: POST
+          path: "/more/stuff.endpoint/{id}"
+          url_param: <
+            name: "id"
+            type: <
+              primitive: STRING
+              source_context: <
+                file: "tests/endpoints.sysl"
+                start: <
+                  line: 6
+                  col: 27
+                >
+                end: <
+                  line: 6
+                  col: 40
+                >
+              >
+            >
+          >
+        >
+      >
+    >
+  >
+>
\ No newline at end of file