[THRIFT-5757] PHP cross test

.github/workflows/build.yml

@@ -167,6 +167,26 @@ jobs:
       - name: Run Tests
         run: vendor/bin/phpunit -c lib/php/phpunit.xml
 
+      - name: Build PHP cross test extension
+        if: matrix.php-version == '8.3'
+        run: make -C lib/php src/ext/thrift_protocol/modules/thrift_protocol.so
+
+      - name: Run make precross for php test
+        if: matrix.php-version == '8.3'
+        run: make -C test/php precross
+
+      - name: Upload php precross artifacts
+        if: matrix.php-version == '8.3'
+        uses: actions/upload-artifact@v7
+        with:
+          name: php-precross
+          if-no-files-found: error
+          path: |
+            lib/php/src/ext/thrift_protocol/modules/thrift_protocol.so
+            test/php/gen-php/
+            test/php/gen-php-classmap/
+          retention-days: 3
+
   lib-go:
     needs: compiler
     runs-on: ubuntu-24.04
@@ -839,6 +859,7 @@ jobs:
 
   cross-test:
     needs:
+      - lib-php
       - lib-java-kotlin
       #- lib-swift                     # currently broken and no maintainers around -> see THRIFT-5864
       #- lib-rust                      # currently broken and no maintainers around -> see THRIFT-5917
@@ -853,9 +874,9 @@ jobs:
         # swift is currently broken and no maintainers around -> see THRIFT-5864
         # rust currently broken and no maintainers around -> see THRIFT-5917
         # kotlin cross test are failing -> see THRIFT-5879
-        server_lang: ['java', 'go', 'cpp', 'py', 'rb', 'nodejs', 'nodets']
+        server_lang: ['java', 'go', 'cpp', 'py', 'rb', 'php', 'nodejs', 'nodets']
         # we always use comma join as many client langs as possible, to reduce the number of jobs
-        client_lang: ['java,kotlin', 'go,cpp,py,nodejs,nodets', 'rb']
+        client_lang: ['java,kotlin', 'go,cpp,py,php,nodejs,nodets', 'rb']
       fail-fast: false
     steps:
       - uses: actions/checkout@v6
@@ -869,6 +890,16 @@ jobs:
           python -m pip install --upgrade pip setuptools
           python -m pip install "tornado>=6.3.0" "twisted>=24.3.0" "zope.interface>=6.1"
 
+      - name: Set up PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: "8.3"
+          extensions: mbstring, intl, xml, curl, sockets
+          ini-values: "error_reporting=E_ALL"
+
+      - name: Install PHP dependencies
+        run: composer install --no-progress --prefer-dist
+
       - uses: actions/setup-java@v5
         with:
           distribution: temurin
@@ -941,6 +972,18 @@ jobs:
           name: rb-precross
           path: .
 
+      - name: Download php precross artifacts
+        uses: actions/download-artifact@v8
+        with:
+          name: php-precross
+          path: .
+
+      - name: Prepare PHP cross test extensions
+        run: |
+          mkdir -p test/php/php_ext_dir
+          ln -sf ../../../lib/php/src/ext/thrift_protocol/modules/thrift_protocol.so test/php/php_ext_dir/
+          ln -sf "$(php-config --extension-dir)/sockets.so" test/php/php_ext_dir/
+
       - name: Download nodejs and nodets precross artifacts
         uses: actions/download-artifact@v8
         with:

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

@@ -1550,11 +1550,10 @@ void t_php_generator::generate_process_function(std::ostream& out, t_service* ts
     out << indent() << "$result = new " << resultname << "();" << '\n';
   }
 
-  // Try block for a function with exceptions
-  if (xceptions.size() > 0) {
-    out << indent() << "try {" << '\n';
-    indent_up();
-  }
+  // Wrap handler invocations so undeclared runtime failures become
+  // TApplicationException responses instead of crashing the PHP test server.
+  out << indent() << "try {" << '\n';
+  indent_up();
 
   // Generate the function call
   t_struct* arg_struct = tfunction->get_arglist();
@@ -1577,23 +1576,44 @@ void t_php_generator::generate_process_function(std::ostream& out, t_service* ts
   }
   out << ");" << '\n';
 
-  if (!tfunction->is_oneway() && xceptions.size() > 0) {
-    indent_down();
-    for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
-      out << indent() << "} catch ("
-                 << php_namespace(get_true_type((*x_iter)->get_type())->get_program())
-                 << (*x_iter)->get_type()->get_name() << " $" << (*x_iter)->get_name() << ") {"
-                 << '\n';
-      if (!tfunction->is_oneway()) {
-        indent_up();
-        out << indent() << "$result->" << (*x_iter)->get_name() << " = $"
-                   << (*x_iter)->get_name() << ";" << '\n';
-        indent_down();
-        out << indent();
-      }
+  indent_down();
+  for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
+    out << indent() << "} catch ("
+               << php_namespace(get_true_type((*x_iter)->get_type())->get_program())
+               << (*x_iter)->get_type()->get_name() << " $" << (*x_iter)->get_name() << ") {"
+               << '\n';
+    if (!tfunction->is_oneway()) {
+      indent_up();
+      out << indent() << "$result->" << (*x_iter)->get_name() << " = $"
+                 << (*x_iter)->get_name() << ";" << '\n';
+      indent_down();
     }
-    out << "}" << '\n';
   }
+  out << indent() << "} catch (TApplicationException $ex) {" << '\n';
+  indent_up();
+  if (!tfunction->is_oneway()) {
+    out << indent() << "$output->writeMessageBegin('" << tfunction->get_name()
+        << "', TMessageType::EXCEPTION, $seqid);" << '\n'
+        << indent() << "$ex->write($output);" << '\n'
+        << indent() << "$output->writeMessageEnd();" << '\n'
+        << indent() << "$output->getTransport()->flush();" << '\n';
+  }
+  out << indent() << "return;" << '\n';
+  indent_down();
+  out << indent() << "} catch (\\Throwable $ex) {" << '\n';
+  indent_up();
+  if (!tfunction->is_oneway()) {
+    out << indent() << "$x = new TApplicationException($ex->getMessage(), "
+        << "TApplicationException::INTERNAL_ERROR);" << '\n'
+        << indent() << "$output->writeMessageBegin('" << tfunction->get_name()
+        << "', TMessageType::EXCEPTION, $seqid);" << '\n'
+        << indent() << "$x->write($output);" << '\n'
+        << indent() << "$output->writeMessageEnd();" << '\n'
+        << indent() << "$output->getTransport()->flush();" << '\n';
+  }
+  out << indent() << "return;" << '\n';
+  indent_down();
+  out << indent() << "}" << '\n';
 
   // Shortcut out here for oneway functions
   if (tfunction->is_oneway()) {
@@ -2707,6 +2727,7 @@ string t_php_generator::declare_field(t_field* tfield, bool init, bool obj) {
       case t_base_type::TYPE_VOID:
         break;
       case t_base_type::TYPE_STRING:
+      case t_base_type::TYPE_UUID:
         result += " = ''";
         break;
       case t_base_type::TYPE_BOOL:

lib/php/Makefile.am

@@ -64,8 +64,8 @@ phpfactory_DATA = \
 	lib/Factory/TJSONProtocolFactory.php \
 	lib/Factory/TProtocolFactory.php \
 	lib/Factory/TStringFuncFactory.php \
-	lib/Factory/TTransportFactoryInterface.php
-	lib/Factory/TTransportFactory.php
+	lib/Factory/TTransportFactoryInterface.php \
+	lib/Factory/TTransportFactory.php \
 	lib/Factory/TFramedTransportFactory.php
 
 phpprotocoldir = $(phpdir)/Protocol
@@ -158,4 +158,3 @@ EXTRA_DIST = \
 
 MAINTAINERCLEANFILES = \
 	Makefile.in
-

lib/php/lib/Protocol/TJSONProtocol.php

@@ -210,7 +210,7 @@ private function writeJSONString($b)
             $this->trans_->write(self::QUOTE);
         }
 
-        $this->trans_->write(json_encode($b, JSON_UNESCAPED_UNICODE));
+        $this->trans_->write(json_encode($b, JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES));
 
         if (is_numeric($b) && $this->context_->escapeNum()) {
             $this->trans_->write(self::QUOTE);

lib/php/lib/Protocol/TSimpleJSONProtocol.php

@@ -85,7 +85,7 @@ private function writeJSONString($b)
     {
         $this->writeContext_->write();
 
-        $this->trans_->write(json_encode((string)$b));
+        $this->trans_->write(json_encode((string)$b, JSON_UNESCAPED_SLASHES));
     }
 
     private function writeJSONInteger($num)

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

@@ -72,6 +72,7 @@ enum TType {
   T_MAP        = 13,
   T_SET        = 14,
   T_LIST       = 15,
+  T_UUID       = 16,
   T_UTF8       = 16,
   T_UTF16      = 17
 };
@@ -85,6 +86,61 @@ const int8_t T_EXCEPTION = 3;
 const int INVALID_DATA = 1;
 const int BAD_VERSION = 4;
 
+static inline int uuid_hex_nibble(char c) {
+  if (c >= '0' && c <= '9') {
+    return c - '0';
+  }
+  if (c >= 'a' && c <= 'f') {
+    return 10 + (c - 'a');
+  }
+  if (c >= 'A' && c <= 'F') {
+    return 10 + (c - 'A');
+  }
+  return -1;
+}
+
+static bool parse_uuid_string(const char* value, size_t len, uint8_t (&uuid)[16]) {
+  size_t nibble_index = 0;
+  int high_nibble = -1;
+
+  for (size_t i = 0; i < len; ++i) {
+    const char c = value[i];
+    if (c == '-' || c == '{' || c == '}') {
+      continue;
+    }
+
+    const int nibble = uuid_hex_nibble(c);
+    if (nibble < 0) {
+      return false;
+    }
+
+    if (high_nibble < 0) {
+      high_nibble = nibble;
+    } else {
+      if (nibble_index >= sizeof(uuid)) {
+        return false;
+      }
+      uuid[nibble_index++] = static_cast<uint8_t>((high_nibble << 4) | nibble);
+      high_nibble = -1;
+    }
+  }
+
+  return nibble_index == sizeof(uuid) && high_nibble < 0;
+}
+
+static void format_uuid_string(const uint8_t (&uuid)[16], char (&buffer)[37]) {
+  static const char hex[] = "0123456789abcdef";
+  size_t out = 0;
+  for (size_t i = 0; i < sizeof(uuid); ++i) {
+    if (i == 4 || i == 6 || i == 8 || i == 10) {
+      buffer[out++] = '-';
+    }
+    buffer[out++] = hex[(uuid[i] >> 4) & 0x0f];
+    buffer[out++] = hex[uuid[i] & 0x0f];
+  }
+  buffer[out] = '\0';
+}
+
 zend_module_entry thrift_protocol_module_entry = {
   STANDARD_MODULE_HEADER,
   "thrift_protocol",
@@ -467,8 +523,10 @@ void skip_element(long thrift_typeID, PHPInputTransport& transport) {
     case T_DOUBLE:
       transport.skip(8);
       return;
+    case T_UUID:
+      transport.skip(16);
+      return;
     //case T_UTF7: // aliases T_STRING
-    case T_UTF8:
     case T_UTF16:
     case T_STRING: {
       uint32_t len = transport.readU32();
@@ -582,7 +640,6 @@ void binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport, zval
       RETURN_DOUBLE(a.d);
     }
     //case T_UTF7: // aliases T_STRING
-    case T_UTF8:
     case T_UTF16:
     case T_STRING: {
       uint32_t size = transport.readU32();
@@ -596,6 +653,14 @@ void binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport, zval
       }
       return;
     }
+    case T_UUID: {
+      uint8_t uuid[16];
+      char strbuf[37];
+      transport.readBytes(uuid, sizeof(uuid));
+      format_uuid_string(uuid, strbuf);
+      ZVAL_STRINGL(return_value, strbuf, sizeof(strbuf) - 1);
+      return;
+    }
     case T_MAP: { // array of key -> value
       uint8_t types[2];
       transport.readBytes(types, 2);
@@ -673,7 +738,7 @@ void binary_deserialize(int8_t thrift_typeID, PHPInputTransport& transport, zval
 
 static
 void binary_serialize_hashtable_key(int8_t keytype, PHPOutputTransport& transport, HashTable* ht, HashPosition& ht_pos, HashTable* spec) {
-  bool keytype_is_numeric = (!((keytype == T_STRING) || (keytype == T_UTF8) || (keytype == T_UTF16)));
+  bool keytype_is_numeric = (!((keytype == T_STRING) || (keytype == T_UUID) || (keytype == T_UTF16)));
 
   zend_string* key;
   uint key_len;
@@ -751,7 +816,15 @@ void binary_serialize(int8_t thrift_typeID, PHPOutputTransport& transport, zval*
       a.d = Z_DVAL_P(value);
       transport.writeI64(a.c);
     } return;
-    case T_UTF8:
+    case T_UUID: {
+      if (Z_TYPE_P(value) != IS_STRING) convert_to_string(value);
+      uint8_t uuid[16];
+      if (!parse_uuid_string(Z_STRVAL_P(value), Z_STRLEN_P(value), uuid)) {
+        throw_tprotocolexception("Attempt to send an invalid UUID string", INVALID_DATA);
+      }
+      transport.write(reinterpret_cast<const char*>(uuid), sizeof(uuid));
+      return;
+    }
     case T_UTF16:
     case T_STRING:
       if (Z_TYPE_P(value) != IS_STRING) convert_to_string(value);