GH-3572: Bump Apache Thrift to 0.23.0 (#3589)

* GH-3572

Bump thrift to 0.23

Added instructions in docs as to where to find the
gpg/sha signatures and a link to the thrift team KEYS file.

* Thrift 0.23 now checks limits (good!) on read write and depth

But it means: if you don't supply a TTransport to TProtocol ctor
you hit an NPE during IO whenever those limits are validated.

Add a StubTTransport which no-ops the read range check and
declares there's no limit on depth checking.

* Spotless

* review feedback

Author: steveloughran

README.md

@@ -43,19 +43,23 @@ Parquet-Java uses Maven to build and depends on the thrift compiler (protoc is n
 To build and install the thrift compiler, run:
 
 ```
-wget -nv https://archive.apache.org/dist/thrift/0.22.0/thrift-0.22.0.tar.gz
-tar xzf thrift-0.22.0.tar.gz
-cd thrift-0.22.0
+wget -nv https://archive.apache.org/dist/thrift/0.23.0/thrift-0.23.0.tar.gz
+tar xzf thrift-0.23.0.tar.gz
+cd thrift-0.23.0
 chmod +x ./configure
 ./configure --disable-libs
 sudo make install -j
 ```
 
-If you're on OSX and use homebrew, you can instead install Thrift 0.22.0 with `brew` and ensure that it comes first in your `PATH`.
+Note: if you wish to verify the signature and checksum of a release:
+1. The GPG and sha checksums can be found under https://archive.apache.org/dist/thrift/0.23.0/
+2. Validate the signature of the artifact against the [Thrift committer KEYS](https://downloads.apache.org/thrift/KEYS).
+
+If you're on OSX and use homebrew, you can instead install Thrift 0.23.0 with `brew` and ensure that it comes first in your `PATH`.
 
 ```
 brew install thrift
-export PATH="/usr/local/opt/thrift@0.22.0/bin:$PATH"
+export PATH="/usr/local/opt/thrift@0.23.0/bin:$PATH"
 ```
 
 ### Build Parquet with Maven

dev/ci-before_install.sh

@@ -20,7 +20,7 @@
 # This script gets invoked by the CI system in a "before install" step
 ################################################################################
 
-export THRIFT_VERSION=0.22.0
+export THRIFT_VERSION=0.23.0
 
 set -e
 set -o pipefail

parquet-thrift/src/main/java/org/apache/parquet/thrift/BufferedProtocolReadToWrite.java

@@ -579,7 +579,7 @@ private void checkEnum(ThriftType expectedType, int i) {
   class NullProtocol extends TProtocol {
 
     public NullProtocol() {
-      super(null);
+      super(StubTTransport.INSTANCE);
     }
 
     @Override

parquet-thrift/src/main/java/org/apache/parquet/thrift/ParquetProtocol.java

@@ -38,7 +38,7 @@ public abstract class ParquetProtocol extends TProtocol {
   private String name;
 
   ParquetProtocol() {
-    super(null);
+    super(StubTTransport.INSTANCE);
   }
 
   private String getClassInfo() {
@@ -54,7 +54,7 @@ private String getClassInfo() {
    * @param name a meaningful debugging name for anonymous inner classes
    */
   public ParquetProtocol(String name) {
-    super(null);
+    super(StubTTransport.INSTANCE);
     this.name = name;
   }
 

parquet-thrift/src/main/java/org/apache/parquet/thrift/StubTTransport.java

@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.parquet.thrift;
+
+import org.apache.thrift.TConfiguration;
+import org.apache.thrift.transport.TTransport;
+import org.apache.thrift.transport.TTransportException;
+
+/**
+ * A stub transport which implements the minimum amount needed for range/depth
+ * validation within the thrift library to succeed.
+ */
+final class StubTTransport extends TTransport {
+
+  static final StubTTransport INSTANCE = new StubTTransport();
+
+  /**
+   * There's no limits on recursion depth.
+   */
+  private static final TConfiguration CONFIGURATION = new TConfiguration(
+      TConfiguration.DEFAULT_MAX_FRAME_SIZE, TConfiguration.DEFAULT_MAX_MESSAGE_SIZE, Integer.MAX_VALUE);
+
+  private StubTTransport() {}
+
+  @Override
+  public void checkReadBytesAvailable(long l) {}
+
+  @Override
+  public boolean isOpen() {
+    return true;
+  }
+
+  @Override
+  public void open() throws TTransportException {
+    throw new TTransportException("unsupported");
+  }
+
+  @Override
+  public void close() {}
+
+  @Override
+  public int read(byte[] bytes, int i, int i1) throws TTransportException {
+    throw new TTransportException("unsupported");
+  }
+
+  @Override
+  public void write(byte[] bytes, int i, int i1) throws TTransportException {
+    throw new TTransportException("unsupported");
+  }
+
+  @Override
+  public TConfiguration getConfiguration() {
+    return CONFIGURATION;
+  }
+
+  @Override
+  public void updateKnownMessageSize(long l) {}
+}

pom.xml

@@ -89,7 +89,7 @@
     <thrift.executable>thrift</thrift.executable>
     <format.thrift.executable>${thrift.executable}</format.thrift.executable>
     <thrift-maven-plugin.version>0.10.0</thrift-maven-plugin.version>
-    <thrift.version>0.22.0</thrift.version>
+    <thrift.version>0.23.0</thrift.version>
     <format.thrift.version>${thrift.version}</format.thrift.version>
     <fastutil.version>8.5.18</fastutil.version>
     <semver.api.version>0.9.33</semver.api.version>