You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by jk...@apache.org on 2017/03/30 20:43:20 UTC
thrift git commit: THRIFT-4126: implement required fields validation
in php extension when validate compiler option is enabled Client: php
Repository: thrift
Updated Branches:
refs/heads/master 7470995ce -> 1360270eb
THRIFT-4126: implement required fields validation in php extension when validate compiler option is enabled
Client: php
This closes #1215
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/1360270e
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/1360270e
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/1360270e
Branch: refs/heads/master
Commit: 1360270eb8e03402d48322514eaa58342e5b25d0
Parents: 7470995
Author: kufd <ko...@ukr.net>
Authored: Sun Mar 19 19:48:50 2017 +0200
Committer: James E. King, III <jk...@apache.org>
Committed: Thu Mar 30 16:42:11 2017 -0400
----------------------------------------------------------------------
.../cpp/src/thrift/generate/t_php_generator.cc | 16 +++----
.../ext/thrift_protocol/php_thrift_protocol.cpp | 44 +++++++++++++++++++-
.../thrift_protocol/php_thrift_protocol7.cpp | 42 +++++++++++++++++++
3 files changed, 91 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/1360270e/compiler/cpp/src/thrift/generate/t_php_generator.cc
----------------------------------------------------------------------
diff --git a/compiler/cpp/src/thrift/generate/t_php_generator.cc b/compiler/cpp/src/thrift/generate/t_php_generator.cc
index 515e165..9b5062c 100644
--- a/compiler/cpp/src/thrift/generate/t_php_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_php_generator.cc
@@ -760,10 +760,7 @@ void t_php_generator::generate_php_type_spec(ofstream& out, t_type* t) {
* type information to generalize serialization routines.
*/
void t_php_generator::generate_php_struct_spec(ofstream& out, t_struct* tstruct) {
- indent(out) << "if (!isset(self::$_TSPEC)) {" << endl;
- indent_up();
-
- indent(out) << "self::$_TSPEC = array(" << endl;
+ indent(out) << "static $_TSPEC = array(" << endl;
indent_up();
const vector<t_field*>& members = tstruct->get_members();
@@ -773,15 +770,14 @@ void t_php_generator::generate_php_struct_spec(ofstream& out, t_struct* tstruct)
indent(out) << (*m_iter)->get_key() << " => array(" << endl;
indent_up();
out << indent() << "'var' => '" << (*m_iter)->get_name() << "'," << endl;
+ out << indent() << "'isRequired' => " << ((*m_iter)->get_req() == t_field::T_REQUIRED ? "true" : "false") << "," << endl;
generate_php_type_spec(out, t);
indent(out) << ")," << endl;
indent_down();
}
indent_down();
- indent(out) << " );" << endl;
- indent_down();
- indent(out) << "}" << endl;
+ indent(out) << " );" << endl << endl;
}
/**
@@ -813,7 +809,9 @@ void t_php_generator::generate_php_struct_definition(ofstream& out,
out << " {" << endl;
indent_up();
- indent(out) << "static $_TSPEC;" << endl << endl;
+ out << indent() << "static $isValidate = " << (validate_ ? "true" : "false") << ";" << endl << endl;
+
+ generate_php_struct_spec(out, tstruct);
for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
string dval = "null";
@@ -832,8 +830,6 @@ void t_php_generator::generate_php_struct_definition(ofstream& out,
out << indent() << "public function __construct(" << param << ") {" << endl;
indent_up();
- generate_php_struct_spec(out, tstruct);
-
if (members.size() > 0) {
for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
t_type* t = get_true_type((*m_iter)->get_type());
http://git-wip-us.apache.org/repos/asf/thrift/blob/1360270e/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp
----------------------------------------------------------------------
diff --git a/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp b/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp
index f9b3ad7..4c9062e 100644
--- a/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp
+++ b/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp
@@ -728,6 +728,42 @@ inline bool ttypes_are_compatible(int8_t t1, int8_t t2) {
return ((t1 == t2) || (ttype_is_int(t1) && ttype_is_int(t2)));
}
+//is used to validate objects before serialization and after deserialization. For now, only required fields are validated.
+static
+void validate_thrift_object(zval* object) {
+
+ HashPosition key_ptr;
+ zval** val_ptr;
+
+ TSRMLS_FETCH();
+ zend_class_entry* object_class_entry = zend_get_class_entry(object TSRMLS_CC);
+ HashTable* spec = Z_ARRVAL_P(zend_read_static_property(object_class_entry, "_TSPEC", 6, false TSRMLS_CC));
+
+ for (zend_hash_internal_pointer_reset_ex(spec, &key_ptr); zend_hash_get_current_data_ex(spec, (void**)&val_ptr, &key_ptr) == SUCCESS; zend_hash_move_forward_ex(spec, &key_ptr)) {
+ ulong fieldno;
+ if (zend_hash_get_current_key_ex(spec, NULL, NULL, &fieldno, 0, &key_ptr) != HASH_KEY_IS_LONG) {
+ throw_tprotocolexception("Bad keytype in TSPEC (expected 'long')", INVALID_DATA);
+ return;
+ }
+ HashTable* fieldspec = Z_ARRVAL_PP(val_ptr);
+
+ // field name
+ zend_hash_find(fieldspec, "var", 4, (void**)&val_ptr);
+ char* varname = Z_STRVAL_PP(val_ptr);
+
+ zend_hash_find(fieldspec, "isRequired", 11, (void**)&val_ptr);
+ bool is_required = Z_BVAL_PP(val_ptr);
+
+ zval* prop = zend_read_property(object_class_entry, object, varname, strlen(varname), false TSRMLS_CC);
+
+ if (is_required && Z_TYPE_P(prop) == IS_NULL) {
+ char errbuf[128];
+ snprintf(errbuf, 128, "Required field %s.%s is unset!", object_class_entry->name, varname);
+ throw_tprotocolexception(errbuf, INVALID_DATA);
+ }
+ }
+}
+
void binary_deserialize_spec(zval* zthis, PHPInputTransport& transport, HashTable* spec) {
// SET and LIST have 'elem' => array('type', [optional] 'class')
// MAP has 'val' => array('type', [optiona] 'class')
@@ -737,7 +773,10 @@ void binary_deserialize_spec(zval* zthis, PHPInputTransport& transport, HashTabl
zval** val_ptr = NULL;
int8_t ttype = transport.readI8();
- if (ttype == T_STOP) return;
+ if (ttype == T_STOP) {
+ validate_thrift_object(zthis);
+ return;
+ }
int16_t fieldno = transport.readI16();
if (zend_hash_index_find(spec, fieldno, (void**)&val_ptr) == SUCCESS) {
HashTable* fieldspec = Z_ARRVAL_PP(val_ptr);
@@ -903,6 +942,9 @@ void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, zval*
void binary_serialize_spec(zval* zthis, PHPOutputTransport& transport, HashTable* spec) {
+
+ validate_thrift_object(zthis);
+
HashPosition key_ptr;
zval** val_ptr;
http://git-wip-us.apache.org/repos/asf/thrift/blob/1360270e/lib/php/src/ext/thrift_protocol/php_thrift_protocol7.cpp
----------------------------------------------------------------------
diff --git a/lib/php/src/ext/thrift_protocol/php_thrift_protocol7.cpp b/lib/php/src/ext/thrift_protocol/php_thrift_protocol7.cpp
index da5b3de..6d8b76f 100644
--- a/lib/php/src/ext/thrift_protocol/php_thrift_protocol7.cpp
+++ b/lib/php/src/ext/thrift_protocol/php_thrift_protocol7.cpp
@@ -849,6 +849,44 @@ bool ttypes_are_compatible(int8_t t1, int8_t t2) {
return ((t1 == t2) || (ttype_is_int(t1) && ttype_is_int(t2)));
}
+//is used to validate objects before serialization and after deserialization. For now, only required fields are validated.
+static
+void validate_thrift_object(zval* object) {
+ zend_class_entry* object_class_entry = Z_OBJCE_P(object);
+ zval* is_validate = zend_read_static_property(object_class_entry, "isValidate", sizeof("isValidate")-1, false);
+ zval* spec = zend_read_static_property(object_class_entry, "_TSPEC", sizeof("_TSPEC")-1, false);
+ HashPosition key_ptr;
+ zval* val_ptr;
+
+ if (Z_TYPE_INFO_P(is_validate) == IS_TRUE) {
+ for (zend_hash_internal_pointer_reset_ex(Z_ARRVAL_P(spec), &key_ptr);
+ (val_ptr = zend_hash_get_current_data_ex(Z_ARRVAL_P(spec), &key_ptr)) != nullptr;
+ zend_hash_move_forward_ex(Z_ARRVAL_P(spec), &key_ptr)) {
+
+ zend_ulong fieldno;
+ if (zend_hash_get_current_key_ex(Z_ARRVAL_P(spec), nullptr, &fieldno, &key_ptr) != HASH_KEY_IS_LONG) {
+ throw_tprotocolexception("Bad keytype in TSPEC (expected 'long')", INVALID_DATA);
+ return;
+ }
+ HashTable* fieldspec = Z_ARRVAL_P(val_ptr);
+
+ // field name
+ zval* zvarname = zend_hash_str_find(fieldspec, "var", sizeof("var")-1);
+ char* varname = Z_STRVAL_P(zvarname);
+
+ zval* is_required = zend_hash_str_find(fieldspec, "isRequired", sizeof("isRequired")-1);
+ zval rv;
+ zval* prop = zend_read_property(object_class_entry, object, varname, strlen(varname), false, &rv);
+
+ if (Z_TYPE_INFO_P(is_required) == IS_TRUE && Z_TYPE_P(prop) == IS_NULL) {
+ char errbuf[128];
+ snprintf(errbuf, 128, "Required field %s.%s is unset!", ZSTR_VAL(object_class_entry->name), varname);
+ throw_tprotocolexception(errbuf, INVALID_DATA);
+ }
+ }
+ }
+}
+
static
void binary_deserialize_spec(zval* zthis, PHPInputTransport& transport, HashTable* spec) {
// SET and LIST have 'elem' => array('type', [optional] 'class')
@@ -857,6 +895,7 @@ void binary_deserialize_spec(zval* zthis, PHPInputTransport& transport, HashTabl
while (true) {
int8_t ttype = transport.readI8();
if (ttype == T_STOP) {
+ validate_thrift_object(zthis);
return;
}
@@ -892,6 +931,9 @@ void binary_deserialize_spec(zval* zthis, PHPInputTransport& transport, HashTabl
static
void binary_serialize_spec(zval* zthis, PHPOutputTransport& transport, HashTable* spec) {
+
+ validate_thrift_object(zthis);
+
HashPosition key_ptr;
zval* val_ptr;