Skip to content

Commit a04a494

Browse files
committed
THRIFT-5757: finish PHP cross-test integration
1 parent 13cdb44 commit a04a494

14 files changed

Lines changed: 276 additions & 80 deletions

File tree

.github/workflows/build.yml

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,26 @@ jobs:
167167
- name: Run Tests
168168
run: vendor/bin/phpunit -c lib/php/phpunit.xml
169169

170+
- name: Build PHP cross test extension
171+
if: matrix.php-version == '8.3'
172+
run: make -C lib/php src/ext/thrift_protocol/modules/thrift_protocol.so
173+
174+
- name: Run make precross for php test
175+
if: matrix.php-version == '8.3'
176+
run: make -C test/php precross
177+
178+
- name: Upload php precross artifacts
179+
if: matrix.php-version == '8.3'
180+
uses: actions/upload-artifact@v7
181+
with:
182+
name: php-precross
183+
if-no-files-found: error
184+
path: |
185+
lib/php/src/ext/thrift_protocol/modules/thrift_protocol.so
186+
test/php/gen-php/
187+
test/php/gen-php-classmap/
188+
retention-days: 3
189+
170190
lib-go:
171191
needs: compiler
172192
runs-on: ubuntu-24.04
@@ -839,6 +859,7 @@ jobs:
839859

840860
cross-test:
841861
needs:
862+
- lib-php
842863
- lib-java-kotlin
843864
#- lib-swift # currently broken and no maintainers around -> see THRIFT-5864
844865
#- lib-rust # currently broken and no maintainers around -> see THRIFT-5917
@@ -853,9 +874,9 @@ jobs:
853874
# swift is currently broken and no maintainers around -> see THRIFT-5864
854875
# rust currently broken and no maintainers around -> see THRIFT-5917
855876
# kotlin cross test are failing -> see THRIFT-5879
856-
server_lang: ['java', 'go', 'cpp', 'py', 'rb', 'nodejs', 'nodets']
877+
server_lang: ['java', 'go', 'cpp', 'py', 'rb', 'php', 'nodejs', 'nodets']
857878
# we always use comma join as many client langs as possible, to reduce the number of jobs
858-
client_lang: ['java,kotlin', 'go,cpp,py,nodejs,nodets', 'rb']
879+
client_lang: ['java,kotlin', 'go,cpp,py,php,nodejs,nodets', 'rb']
859880
fail-fast: false
860881
steps:
861882
- uses: actions/checkout@v6
@@ -869,6 +890,16 @@ jobs:
869890
python -m pip install --upgrade pip setuptools
870891
python -m pip install "tornado>=6.3.0" "twisted>=24.3.0" "zope.interface>=6.1"
871892
893+
- name: Set up PHP
894+
uses: shivammathur/setup-php@v2
895+
with:
896+
php-version: "8.3"
897+
extensions: mbstring, intl, xml, curl, sockets
898+
ini-values: "error_reporting=E_ALL"
899+
900+
- name: Install PHP dependencies
901+
run: composer install --no-progress --prefer-dist
902+
872903
- uses: actions/setup-java@v5
873904
with:
874905
distribution: temurin
@@ -941,6 +972,18 @@ jobs:
941972
name: rb-precross
942973
path: .
943974

975+
- name: Download php precross artifacts
976+
uses: actions/download-artifact@v8
977+
with:
978+
name: php-precross
979+
path: .
980+
981+
- name: Prepare PHP cross test extensions
982+
run: |
983+
mkdir -p test/php/php_ext_dir
984+
ln -sf ../../../lib/php/src/ext/thrift_protocol/modules/thrift_protocol.so test/php/php_ext_dir/
985+
ln -sf "$(php-config --extension-dir)/sockets.so" test/php/php_ext_dir/
986+
944987
- name: Download nodejs and nodets precross artifacts
945988
uses: actions/download-artifact@v8
946989
with:

compiler/cpp/src/thrift/generate/t_php_generator.cc

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1550,11 +1550,10 @@ void t_php_generator::generate_process_function(std::ostream& out, t_service* ts
15501550
out << indent() << "$result = new " << resultname << "();" << '\n';
15511551
}
15521552

1553-
// Try block for a function with exceptions
1554-
if (xceptions.size() > 0) {
1555-
out << indent() << "try {" << '\n';
1556-
indent_up();
1557-
}
1553+
// Wrap handler invocations so undeclared runtime failures become
1554+
// TApplicationException responses instead of crashing the PHP test server.
1555+
out << indent() << "try {" << '\n';
1556+
indent_up();
15581557

15591558
// Generate the function call
15601559
t_struct* arg_struct = tfunction->get_arglist();
@@ -1577,23 +1576,44 @@ void t_php_generator::generate_process_function(std::ostream& out, t_service* ts
15771576
}
15781577
out << ");" << '\n';
15791578

1580-
if (!tfunction->is_oneway() && xceptions.size() > 0) {
1581-
indent_down();
1582-
for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
1583-
out << indent() << "} catch ("
1584-
<< php_namespace(get_true_type((*x_iter)->get_type())->get_program())
1585-
<< (*x_iter)->get_type()->get_name() << " $" << (*x_iter)->get_name() << ") {"
1586-
<< '\n';
1587-
if (!tfunction->is_oneway()) {
1588-
indent_up();
1589-
out << indent() << "$result->" << (*x_iter)->get_name() << " = $"
1590-
<< (*x_iter)->get_name() << ";" << '\n';
1591-
indent_down();
1592-
out << indent();
1593-
}
1579+
indent_down();
1580+
for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
1581+
out << indent() << "} catch ("
1582+
<< php_namespace(get_true_type((*x_iter)->get_type())->get_program())
1583+
<< (*x_iter)->get_type()->get_name() << " $" << (*x_iter)->get_name() << ") {"
1584+
<< '\n';
1585+
if (!tfunction->is_oneway()) {
1586+
indent_up();
1587+
out << indent() << "$result->" << (*x_iter)->get_name() << " = $"
1588+
<< (*x_iter)->get_name() << ";" << '\n';
1589+
indent_down();
15941590
}
1595-
out << "}" << '\n';
15961591
}
1592+
out << indent() << "} catch (TApplicationException $ex) {" << '\n';
1593+
indent_up();
1594+
if (!tfunction->is_oneway()) {
1595+
out << indent() << "$output->writeMessageBegin('" << tfunction->get_name()
1596+
<< "', TMessageType::EXCEPTION, $seqid);" << '\n'
1597+
<< indent() << "$ex->write($output);" << '\n'
1598+
<< indent() << "$output->writeMessageEnd();" << '\n'
1599+
<< indent() << "$output->getTransport()->flush();" << '\n';
1600+
}
1601+
out << indent() << "return;" << '\n';
1602+
indent_down();
1603+
out << indent() << "} catch (\\Throwable $ex) {" << '\n';
1604+
indent_up();
1605+
if (!tfunction->is_oneway()) {
1606+
out << indent() << "$x = new TApplicationException($ex->getMessage(), "
1607+
<< "TApplicationException::INTERNAL_ERROR);" << '\n'
1608+
<< indent() << "$output->writeMessageBegin('" << tfunction->get_name()
1609+
<< "', TMessageType::EXCEPTION, $seqid);" << '\n'
1610+
<< indent() << "$x->write($output);" << '\n'
1611+
<< indent() << "$output->writeMessageEnd();" << '\n'
1612+
<< indent() << "$output->getTransport()->flush();" << '\n';
1613+
}
1614+
out << indent() << "return;" << '\n';
1615+
indent_down();
1616+
out << indent() << "}" << '\n';
15971617

15981618
// Shortcut out here for oneway functions
15991619
if (tfunction->is_oneway()) {
@@ -2707,6 +2727,7 @@ string t_php_generator::declare_field(t_field* tfield, bool init, bool obj) {
27072727
case t_base_type::TYPE_VOID:
27082728
break;
27092729
case t_base_type::TYPE_STRING:
2730+
case t_base_type::TYPE_UUID:
27102731
result += " = ''";
27112732
break;
27122733
case t_base_type::TYPE_BOOL:

lib/php/Makefile.am

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ phpfactory_DATA = \
6464
lib/Factory/TJSONProtocolFactory.php \
6565
lib/Factory/TProtocolFactory.php \
6666
lib/Factory/TStringFuncFactory.php \
67-
lib/Factory/TTransportFactoryInterface.php
68-
lib/Factory/TTransportFactory.php
67+
lib/Factory/TTransportFactoryInterface.php \
68+
lib/Factory/TTransportFactory.php \
6969
lib/Factory/TFramedTransportFactory.php
7070

7171
phpprotocoldir = $(phpdir)/Protocol
@@ -158,4 +158,3 @@ EXTRA_DIST = \
158158

159159
MAINTAINERCLEANFILES = \
160160
Makefile.in
161-

lib/php/lib/Protocol/TJSONProtocol.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ private function writeJSONString($b)
210210
$this->trans_->write(self::QUOTE);
211211
}
212212

213-
$this->trans_->write(json_encode($b, JSON_UNESCAPED_UNICODE));
213+
$this->trans_->write(json_encode($b, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES));
214214

215215
if (is_numeric($b) && $this->context_->escapeNum()) {
216216
$this->trans_->write(self::QUOTE);

lib/php/lib/Protocol/TSimpleJSONProtocol.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ private function writeJSONString($b)
8585
{
8686
$this->writeContext_->write();
8787

88-
$this->trans_->write(json_encode((string)$b));
88+
$this->trans_->write(json_encode((string)$b, JSON_UNESCAPED_SLASHES));
8989
}
9090

9191
private function writeJSONInteger($num)

lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp

Lines changed: 77 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ enum TType {
7272
T_MAP = 13,
7373
T_SET = 14,
7474
T_LIST = 15,
75+
T_UUID = 16,
7576
T_UTF8 = 16,
7677
T_UTF16 = 17
7778
};
@@ -85,6 +86,61 @@ const int8_t T_EXCEPTION = 3;
8586
const int INVALID_DATA = 1;
8687
const int BAD_VERSION = 4;
8788

89+
static inline int uuid_hex_nibble(char c) {
90+
if (c >= '0' && c <= '9') {
91+
return c - '0';
92+
}
93+
if (c >= 'a' && c <= 'f') {
94+
return 10 + (c - 'a');
95+
}
96+
if (c >= 'A' && c <= 'F') {
97+
return 10 + (c - 'A');
98+
}
99+
return -1;
100+
}
101+
102+
static bool parse_uuid_string(const char* value, size_t len, uint8_t (&uuid)[16]) {
103+
size_t nibble_index = 0;
104+
int high_nibble = -1;
105+
106+
for (size_t i = 0; i < len; ++i) {
107+
const char c = value[i];
108+
if (c == '-' || c == '{' || c == '}') {
109+
continue;
110+
}
111+
112+
const int nibble = uuid_hex_nibble(c);
113+
if (nibble < 0) {
114+
return false;
115+
}
116+
117+
if (high_nibble < 0) {
118+
high_nibble = nibble;
119+
} else {
120+
if (nibble_index >= sizeof(uuid)) {
121+
return false;
122+
}
123+
uuid[nibble_index++] = static_cast<uint8_t>((high_nibble << 4) | nibble);
124+
high_nibble = -1;
125+
}
126+
}
127+
128+
return nibble_index == sizeof(uuid) && high_nibble < 0;
129+
}
130+
131+
static void format_uuid_string(const uint8_t (&uuid)[16], char (&buffer)[37]) {
132+
static const char hex[] = "0123456789abcdef";
133+
size_t out = 0;
134+
for (size_t i = 0; i < sizeof(uuid); ++i) {
135+
if (i == 4 || i == 6 || i == 8 || i == 10) {
136+
buffer[out++] = '-';
137+
}
138+
buffer[out++] = hex[(uuid[i] >> 4) & 0x0f];
139+
buffer[out++] = hex[uuid[i] & 0x0f];
140+
}
141+
buffer[out] = '\0';
142+
}
143+
88144
zend_module_entry thrift_protocol_module_entry = {
89145
STANDARD_MODULE_HEADER,
90146
"thrift_protocol",
@@ -467,8 +523,10 @@ void skip_element(long thrift_typeID, PHPInputTransport& transport) {
467523
case T_DOUBLE:
468524
transport.skip(8);
469525
return;
526+
case T_UUID:
527+
transport.skip(16);
528+
return;
470529
//case T_UTF7: // aliases T_STRING
471-
case T_UTF8:
472530
case T_UTF16:
473531
case T_STRING: {
474532
uint32_t len = transport.readU32();
@@ -582,7 +640,6 @@ void binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport, zval
582640
RETURN_DOUBLE(a.d);
583641
}
584642
//case T_UTF7: // aliases T_STRING
585-
case T_UTF8:
586643
case T_UTF16:
587644
case T_STRING: {
588645
uint32_t size = transport.readU32();
@@ -596,6 +653,14 @@ void binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport, zval
596653
}
597654
return;
598655
}
656+
case T_UUID: {
657+
uint8_t uuid[16];
658+
char strbuf[37];
659+
transport.readBytes(uuid, sizeof(uuid));
660+
format_uuid_string(uuid, strbuf);
661+
ZVAL_STRINGL(return_value, strbuf, sizeof(strbuf) - 1);
662+
return;
663+
}
599664
case T_MAP: { // array of key -> value
600665
uint8_t types[2];
601666
transport.readBytes(types, 2);
@@ -673,7 +738,7 @@ void binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport, zval
673738

674739
static
675740
void binary_serialize_hashtable_key(int8_t keytype, PHPOutputTransport& transport, HashTable* ht, HashPosition& ht_pos, HashTable* spec) {
676-
bool keytype_is_numeric = (!((keytype == T_STRING) || (keytype == T_UTF8) || (keytype == T_UTF16)));
741+
bool keytype_is_numeric = (!((keytype == T_STRING) || (keytype == T_UUID) || (keytype == T_UTF16)));
677742

678743
zend_string* key;
679744
uint key_len;
@@ -751,7 +816,15 @@ void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, zval*
751816
a.d = Z_DVAL_P(value);
752817
transport.writeI64(a.c);
753818
} return;
754-
case T_UTF8:
819+
case T_UUID: {
820+
if (Z_TYPE_P(value) != IS_STRING) convert_to_string(value);
821+
uint8_t uuid[16];
822+
if (!parse_uuid_string(Z_STRVAL_P(value), Z_STRLEN_P(value), uuid)) {
823+
throw_tprotocolexception("Attempt to send an invalid UUID string", INVALID_DATA);
824+
}
825+
transport.write(reinterpret_cast<const char*>(uuid), sizeof(uuid));
826+
return;
827+
}
755828
case T_UTF16:
756829
case T_STRING:
757830
if (Z_TYPE_P(value) != IS_STRING) convert_to_string(value);

0 commit comments

Comments
 (0)