Add new input validator API functions #1576

LaszloLango · 2017-02-10T11:45:36Z

JerryScript-DCO-1.0-Signed-off-by: László Langó [email protected]

robertsipka · 2017-02-10T12:01:41Z

LGTM (informally).

zherczeg · 2017-02-10T13:35:43Z

jerry-core/jerry.c

@@ -256,6 +256,11 @@ jerry_parse (const jerry_char_t *source_p, /**< script source */
 #ifdef JERRY_JS_PARSER
  jerry_assert_api_available ();

+  if (!lit_is_utf8_string_valid ((lit_utf8_byte_t *) source_p, (lit_utf8_size_t) source_size))
+  {
+    return ecma_raise_syntax_error (ECMA_ERR_MSG ("The input should be a valid UTF8 string."));


Input must be a valid UTF-8 string.

zherczeg · 2017-02-10T13:36:08Z

jerry-core/jerry-snapshot.c

+  {
+    return 0;
+  }
+


Do we need this? jerry_parse_and_save_snapshot calls jerry_parse.

Yes we do. It does not call jerry_parse. It calls parser_parse_script

Perhaps this is really not needed anymore.

zherczeg

LGTM

zherczeg · 2017-02-13T04:59:47Z

I have been thinking about this in the weekend. Perhaps we should make this a runtime enable.disable feature, and do check for all functions which accepts utf8/cesu8 input. This patch is more like a workaround than a fix.

LaszloLango · 2017-02-15T14:53:52Z

@zherczeg, @robertsipka I've updated the PR. Please review again.

zherczeg · 2017-02-16T07:39:18Z

docs/02.API-REFERENCE.md

 - [jerry_get_utf8_string_size](#jerry_get_utf8_string_size)
 - [jerry_get_utf8_string_length](#jerry_get_utf8_string_length)
+- [jerry_is_utf8_string_valid](#jerry_is_utf8_string_valid)
+
+


Do we need two newlines here?

Yes, we do. This is the style in the whole MD

zherczeg · 2017-02-16T07:40:07Z

docs/02.API-REFERENCE.md

+
+```c
+bool
+jerry_is_utf8_string_valid (const jerry_char_t *utf8_buf_p, /**< UTF-8 string */


What do you think of jerry_is_valid_utf8_string? Somehow that is more natural to me.

Good to me. Should we rename the internal functions too? lit_is_valid_utf8_string?

At some point we should. If you wish you can do it in this patch.

Ok, I'll do it in this patch.

zherczeg · 2017-02-16T07:41:29Z

jerry-core/jerry-snapshot.c

+  {
+    return 0;
+  }
+


Perhaps this is really not needed anymore.

zherczeg · 2017-02-16T07:42:38Z

docs/02.API-REFERENCE.md

+
+**See also**
+
+- [jerry_run_simple](#jerry_run_simple)


jerry_run_simple expects utf8 string, not cesu8

Fixed, removed the reference from jerry_is_cesu8_string_valid

zherczeg

LGTM after the rename.

zherczeg · 2017-02-16T12:46:40Z

jerry-core/jerry.c

+ *         false - otherwise
+ */
+bool
+jerry_is_utf8_string_valid (const jerry_char_t *utf8_buf_p, /**< UTF-8 string */


Please rename this function as well (jerry_is_valid_utf8_string)

zherczeg · 2017-02-16T12:46:54Z

jerry-core/jerryscript.h

@@ -330,6 +330,12 @@ bool jerry_foreach_object_property (const jerry_value_t obj_val, jerry_object_pr
                                    void *user_data_p);

 /**
+ * Input validatator functions
+ */
+bool jerry_is_utf8_string_valid (const jerry_char_t *utf8_buf_p, jerry_size_t buf_size);


Here as well.

bzsolt · 2017-02-16T13:18:37Z

docs/02.API-REFERENCE.md

@@ -3287,6 +3309,99 @@ bool foreach_function (const jerry_value_t prop_name,
 - [jerry_object_property_foreach_t](#jerry_object_property_foreach_t)


+# Input validatator functions


validator

bzsolt · 2017-02-16T13:20:07Z

jerry-core/jerryscript.h

@@ -330,6 +330,12 @@ bool jerry_foreach_object_property (const jerry_value_t obj_val, jerry_object_pr
                                    void *user_data_p);

 /**
+ * Input validatator functions


validator

Fixes jerryscript-project#1549 JerryScript-DCO-1.0-Signed-off-by: László Langó [email protected]

dbatyai

LGTM

LaszloLango added the bug Undesired behaviour label Feb 10, 2017

LaszloLango force-pushed the fix-issue-1549 branch from fbfedef to 1503d7d Compare February 10, 2017 12:19

zherczeg requested changes Feb 10, 2017

View reviewed changes

LaszloLango force-pushed the fix-issue-1549 branch from 1503d7d to ca54bf4 Compare February 10, 2017 13:41

zherczeg approved these changes Feb 10, 2017

View reviewed changes

LaszloLango force-pushed the fix-issue-1549 branch 3 times, most recently from 83b0356 to 3c1c77a Compare February 15, 2017 14:50

LaszloLango changed the title ~~Add input validation to 'jerry_parse' and 'jerry_parse_and_save_snapshot'~~ Add new input validator API functions Feb 15, 2017

zherczeg requested changes Feb 16, 2017

View reviewed changes

LaszloLango force-pushed the fix-issue-1549 branch 2 times, most recently from cae38f4 to 755a1e8 Compare February 16, 2017 08:08

zherczeg approved these changes Feb 16, 2017

View reviewed changes

bzsolt reviewed Feb 16, 2017

View reviewed changes

Add new input validator API functions

70a562f

Fixes jerryscript-project#1549 JerryScript-DCO-1.0-Signed-off-by: László Langó [email protected]

LaszloLango force-pushed the fix-issue-1549 branch from 755a1e8 to 70a562f Compare February 16, 2017 13:41

dbatyai approved these changes Feb 16, 2017

View reviewed changes

bzsolt approved these changes Feb 16, 2017

View reviewed changes

dbatyai merged commit 799726a into jerryscript-project:master Feb 16, 2017

LaszloLango deleted the fix-issue-1549 branch April 28, 2017 08:30

LaszloLango mentioned this pull request Aug 30, 2017

String parsing issue: validity checks only in debug builds (002) #1971

Closed

zherczeg mentioned this pull request Mar 27, 2018

Nominating Robert Sipka (robertsipka) to become a reviewer #2254

Closed

		@@ -3287,6 +3309,99 @@ bool foreach_function (const jerry_value_t prop_name,
		- [jerry_object_property_foreach_t](#jerry_object_property_foreach_t)


		# Input validatator functions

Add new input validator API functions #1576

Add new input validator API functions #1576

Uh oh!

Conversation

LaszloLango commented Feb 10, 2017

Uh oh!

robertsipka commented Feb 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

zherczeg commented Feb 13, 2017

Uh oh!

LaszloLango commented Feb 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbatyai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!