From 91768f9c00e9e283e2f49e19cb802fcffed58770 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20R=C3=A5nge?= Date: Fri, 2 Oct 2020 08:42:53 +0000 Subject: [PATCH 1/3] jslt-152: parse-url error handling control See: https://github.com/schibsted/jslt/issues/152 --- .../spt/data/jslt/impl/BuiltinFunctions.java | 21 +++++++++--- core/src/test/resources/query-tests.yaml | 33 +++++++++++++++++++ functions.md | 9 ++++- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java b/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java index 80dd4f48..12bec444 100644 --- a/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java +++ b/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java @@ -1265,16 +1265,18 @@ else if (ComparisonOperator.compare(arguments[0], arguments[1], null) > 0) // ===== PARSE-URL public static class ParseUrl extends AbstractFunction { - public ParseUrl() { super("parse-url", 1,1);} + public ParseUrl() { super("parse-url", 1, 2); } public JsonNode call(JsonNode input, JsonNode[] arguments) { - if (arguments[0].isNull()) + if (arguments.length < 1 || arguments[0].isNull()) return NullNode.instance; String urlString = arguments[0].asText(); + boolean throwOnFailure = arguments.length < 2 || arguments[1].asBoolean(); try { - URL aURL = new URL(arguments[0].asText()); + + URL aURL = new URL(urlString); final ObjectNode objectNode = NodeUtils.mapper.createObjectNode(); if (aURL.getHost() != null && !aURL.getHost().isEmpty()) objectNode.put("host", aURL.getHost()); @@ -1303,8 +1305,17 @@ public JsonNode call(JsonNode input, JsonNode[] arguments) { if(aURL.getUserInfo() != null && !aURL.getUserInfo().isEmpty()) objectNode.put("userinfo", aURL.getUserInfo()); return objectNode; - } catch (MalformedURLException | UnsupportedEncodingException e) { - throw new JsltException("Can't parse " + urlString, e); + } catch (Throwable e) { + if (throwOnFailure) { + throw new JsltException("Can't parse " + urlString, e); + } else { + final ObjectNode errorNode = NodeUtils.mapper.createObjectNode(); + Class errorClass = e.getClass(); + errorNode.put("error", errorClass.getSimpleName()); + errorNode.put("message", e.getMessage()); + errorNode.put("input", urlString); + return errorNode; + } } } } diff --git a/core/src/test/resources/query-tests.yaml b/core/src/test/resources/query-tests.yaml index 8014605a..1c6a1b7b 100644 --- a/core/src/test/resources/query-tests.yaml +++ b/core/src/test/resources/query-tests.yaml @@ -460,3 +460,36 @@ tests: "port": 8443, "scheme": "https" } + +# parse-url tests with throwOnFailure set to true should behave as above + + - input: + query: parse-url("https://example.com:8443", true) + output: | + { + "host": "example.com", + "port": 8443, + "scheme": "https" + } + +# parse-url tests with throwOnFailure set to false should behave as above for good cases + + - input: + query: parse-url("https://example.com:8443", false) + output: | + { + "host": "example.com", + "port": 8443, + "scheme": "https" + } + +# parse-url tests with throwOnFailure set to false should return an error object for bad cases + + - input: + query: parse-url("this-is-an-invalid-url", false) + output: | + { + "error": "MalformedURLException", + "message": "no protocol: this-is-an-invalid-url", + "input": "this-is-an-invalid-url" + } diff --git a/functions.md b/functions.md index a9d9ba80..9e79a00a 100644 --- a/functions.md +++ b/functions.md @@ -826,10 +826,13 @@ format-time(null, "yyyy-MM-dd") => null ## Miscellaneous functions -### _parse-url(url) -> object_ +### _parse-url(url, throwOnFailure?) -> object_ Parses `url` and returns an object with keys [`scheme`, `userinfo`, `host`, `port` `path`, `query`, `parameters`, `fragment` ] +If the optional `throwOnFailure` argument is not specified invalid URLs will generate an exception. If `throwOnFailure` is set to `false` the `parse-url` returns +an object indicating an error occurred. + ``` parse-url("http://example.com").scheme => "http" parse-url("http://example.com").host => "example.com" @@ -841,4 +844,8 @@ parse-url("https://www.example.com/?aa=1&aa=2&bb=&cc").parameters.bb => [null] parse-url("https://www.example.com/?aa=1&aa=2&bb=&cc").parameters.cc => [null] parse-url("ftp://username:password@host.com/").userinfo => "username:password" parse-url("https://example.com:8443").port => 8443 + +parse-url("this-is-an-invalid-url", false).error => "MalformedURLException" +parse-url("this-is-an-invalid-url", false).message => "no protocol: this-is-an-invalid-url" +parse-url("this-is-an-invalid-url", false).input => "this-is-an-invalid-url" ``` From 3b416d8c37740c69b8138a39ec3413b4ffc6278f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20R=C3=A5nge?= Date: Fri, 2 Oct 2020 12:36:31 +0000 Subject: [PATCH 2/3] jslt-152: Addressing documentation feedback --- .../spt/data/jslt/impl/BuiltinFunctions.java | 6 ++-- core/src/test/resources/query-tests.yaml | 2 +- functions.md | 31 ++++++++++++++++++- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java b/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java index 12bec444..27b0caab 100644 --- a/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java +++ b/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java @@ -1305,13 +1305,13 @@ public JsonNode call(JsonNode input, JsonNode[] arguments) { if(aURL.getUserInfo() != null && !aURL.getUserInfo().isEmpty()) objectNode.put("userinfo", aURL.getUserInfo()); return objectNode; - } catch (Throwable e) { + } catch (Exception e) { if (throwOnFailure) { throw new JsltException("Can't parse " + urlString, e); } else { final ObjectNode errorNode = NodeUtils.mapper.createObjectNode(); - Class errorClass = e.getClass(); - errorNode.put("error", errorClass.getSimpleName()); + final Class errorClass = e.getClass(); + errorNode.put("error", errorClass.getName()); errorNode.put("message", e.getMessage()); errorNode.put("input", urlString); return errorNode; diff --git a/core/src/test/resources/query-tests.yaml b/core/src/test/resources/query-tests.yaml index 1c6a1b7b..c2e47c01 100644 --- a/core/src/test/resources/query-tests.yaml +++ b/core/src/test/resources/query-tests.yaml @@ -489,7 +489,7 @@ tests: query: parse-url("this-is-an-invalid-url", false) output: | { - "error": "MalformedURLException", + "error": "java.net.MalformedURLException", "message": "no protocol: this-is-an-invalid-url", "input": "this-is-an-invalid-url" } diff --git a/functions.md b/functions.md index 9e79a00a..67d2c6ae 100644 --- a/functions.md +++ b/functions.md @@ -833,6 +833,35 @@ Parses `url` and returns an object with keys [`scheme`, `userinfo`, `host`, `por If the optional `throwOnFailure` argument is not specified invalid URLs will generate an exception. If `throwOnFailure` is set to `false` the `parse-url` returns an object indicating an error occurred. +#### Example of valid URL response + +```json +{ + "host": "example.com", + "port": 8080, + "path": "/examples/", + "scheme": "https", + "query": "x=1&y=", + "parameters": { + "x": ["1"], + "y": [null] + }, + "fragment": "footer", + "userinfo": "myname:mypwd" +} +``` + +#### Example of invalid URL response with throwOnFailure=false + +```json +{ + "error": "java.net.MalformedURLException", + "message": "no protocol: this-is-an-invalid-url", + "input": "this-is-an-invalid-url" +} +``` + +#### Examples ``` parse-url("http://example.com").scheme => "http" parse-url("http://example.com").host => "example.com" @@ -845,7 +874,7 @@ parse-url("https://www.example.com/?aa=1&aa=2&bb=&cc").parameters.cc => [null] parse-url("ftp://username:password@host.com/").userinfo => "username:password" parse-url("https://example.com:8443").port => 8443 -parse-url("this-is-an-invalid-url", false).error => "MalformedURLException" +parse-url("this-is-an-invalid-url", false).error => "java.net.MalformedURLException" parse-url("this-is-an-invalid-url", false).message => "no protocol: this-is-an-invalid-url" parse-url("this-is-an-invalid-url", false).input => "this-is-an-invalid-url" ``` From fb71daf30547448efadcadf800c8e1a0f98b8044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20R=C3=A5nge?= Date: Sat, 3 Oct 2020 07:44:11 +0000 Subject: [PATCH 3/3] jslt-152: Test cases for failure modes, checks booleaness of throwOnFailure arg --- .../spt/data/jslt/impl/BuiltinFunctions.java | 14 ++++++++++++-- core/src/test/resources/function-error-tests.json | 15 +++++++++++++++ core/src/test/resources/query-tests.yaml | 8 +++++++- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java b/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java index 27b0caab..018031ee 100644 --- a/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java +++ b/core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java @@ -1268,11 +1268,21 @@ public static class ParseUrl extends AbstractFunction { public ParseUrl() { super("parse-url", 1, 2); } public JsonNode call(JsonNode input, JsonNode[] arguments) { - if (arguments.length < 1 || arguments[0].isNull()) + if (arguments[0].isNull()) return NullNode.instance; String urlString = arguments[0].asText(); - boolean throwOnFailure = arguments.length < 2 || arguments[1].asBoolean(); + + // Second argument (throwOnFailure) is optional and defaults to true + boolean throwOnFailure = true; + if (!(arguments.length < 2)) { + JsonNode arg1 = arguments[1]; + if (arg1.isBoolean()) { + throwOnFailure = arg1.asBoolean(); + } else { + throw new JsltException("throwOnFailure argument must be a boolean value"); + } + } try { diff --git a/core/src/test/resources/function-error-tests.json b/core/src/test/resources/function-error-tests.json index e3704714..77ed759b 100644 --- a/core/src/test/resources/function-error-tests.json +++ b/core/src/test/resources/function-error-tests.json @@ -211,6 +211,21 @@ "query" : "index-of(\"abc\", \"b\")", "input" : "null", "error" : "must be an array" + }, + { + "query" : "parse-url(\"invalid-url\")", + "input" : "null", + "error" : "Can't parse invalid-url" + }, + { + "query" : "parse-url(\"invalid-url\", true)", + "input" : "null", + "error" : "Can't parse invalid-url" + }, + { + "query" : "parse-url(\"https://google.com\", 1)", + "input" : "null", + "error" : "throwOnFailure argument must be a boolean value" } ] } diff --git a/core/src/test/resources/query-tests.yaml b/core/src/test/resources/query-tests.yaml index c2e47c01..abb4490d 100644 --- a/core/src/test/resources/query-tests.yaml +++ b/core/src/test/resources/query-tests.yaml @@ -371,6 +371,11 @@ tests: # parse-url tests + - input: + query: parse-url(null) + output: | + null + - input: query: parse-url("https://www.aftonbladet.se/nyheter/a/opyK1R/snosmocka-pa-ingang--kan-bli-20-centimeter?utm_source=aaaa&utm_campaign=bbbb&utm_medium=cccc&utm_content=dddd&utm_term=eeee") output: | @@ -399,6 +404,7 @@ tests: "fragment": "foo", "scheme": "http" } + - input: query: parse-url("http://example.com") output: | @@ -409,6 +415,7 @@ tests: description: - No path - + - input: query: parse-url("http://example.com/") output: | @@ -420,7 +427,6 @@ tests: description: - If URL has an ending slash then it has a path - - input: query: parse-url("https://www.example.com/?aa=1&aa=2&bb=&cc") output: |