Skip to content

Commit

Permalink
finagle-core: Remove Deprecated Transport.peerCertificate Method
Browse files Browse the repository at this point in the history
Problem / Solution

The `Transport.peerCertificate` method (not the Finagle context of the
same name) is deprecated in favor of `TransportContext.peerCertificate`.
This commit removes the deprecated method.

JIRA Issues: CSL-7324

Differential Revision: https://phabricator.twitter.biz/D250027
  • Loading branch information
ryanoneill authored and jenkins committed Dec 10, 2018
1 parent 29d80c8 commit ab0432b
Show file tree
Hide file tree
Showing 12 changed files with 6 additions and 44 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ Breaking API Changes
these methods within `TransportContext` should be changed to use the corresponding
methods on `c.t.f.t.Transport` instead. ``PHAB_ID=D244742``

* finagle-core: The deprecated `c.t.f.t.Transport.peerCertificate` method on the `Transport` class
(not the `Transport.peerCertificate` Finagle context) has been removed. Uses of this
method should be changed to use `c.t.f.t.TransportContext.peerCertificate` instead.
``PHAB_ID=D250027``

* finagle-core: The deprecated `c.t.f.t.TransportContext.status` method has been removed
from `TransportContext`. Uses of this method should be changed to use
`c.t.f.t.Transport.status` instead. ``PHAB_ID=D247234``
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private[finagle] abstract class GenStreamingSerialServerDispatcher[Req, Rep, In,
val save = Local.save()
val dispatched = try {
Contexts.local.let(RemoteInfo.Upstream.AddressCtx, trans.remoteAddress) {
trans.peerCertificate match {
trans.context.peerCertificate match {
case None => dispatch(req)
case Some(cert) =>
Contexts.local.let(Transport.peerCertCtx, cert) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import com.twitter.finagle.Status
import com.twitter.finagle.transport.Transport
import com.twitter.util.{Future, Time}
import java.net.SocketAddress
import java.security.cert.Certificate

/**
* A multi-part object with a single read handle, and a future that is satisfied
Expand Down Expand Up @@ -44,6 +43,5 @@ private[finagle] abstract class StreamTransportProxy[In, Out](val self: Transpor
def localAddress: SocketAddress = self.localAddress
def remoteAddress: SocketAddress = self.remoteAddress
def close(deadline: Time): Future[Unit] = self.close(deadline)
def peerCertificate: Option[Certificate] = self.peerCertificate
def context: Context = self.context
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,6 @@ trait Transport[In, Out] extends Closable { self =>
@deprecated("Please use Transport.context.remoteAddress instead", "2017-08-21")
def remoteAddress: SocketAddress

/**
* The peer certificate if a TLS session is established.
*/
@deprecated("Please use Transport.context.peerCertificate instead", "2017-08-21")
def peerCertificate: Option[Certificate]

/**
* Maps this transport to `Transport[In1, Out2]`. Note, exceptions
* in `f` and `g` are lifted to a [[com.twitter.util.Future]].
Expand All @@ -88,7 +82,6 @@ trait Transport[In, Out] extends Closable { self =>
def onClose: Future[Throwable] = self.onClose
def localAddress: SocketAddress = self.localAddress
def remoteAddress: SocketAddress = self.remoteAddress
def peerCertificate: Option[Certificate] = self.peerCertificate
def close(deadline: Time): Future[Unit] = self.close(deadline)
def context: Context = self.context
override def toString: String = self.toString
Expand Down Expand Up @@ -336,7 +329,6 @@ object Transport {
def onClose: Future[Throwable] = trans.onClose
def localAddress: SocketAddress = trans.localAddress
def remoteAddress: SocketAddress = trans.remoteAddress
def peerCertificate: Option[Certificate] = trans.peerCertificate
def close(deadline: Time): Future[Unit] = trans.close(deadline)
def context: Context = trans.context.asInstanceOf[Context]
override def toString: String = trans.toString
Expand Down Expand Up @@ -375,7 +367,6 @@ abstract class TransportProxy[In, Out](_self: Transport[In, Out]) extends Transp
def onClose: Future[Throwable] = self.onClose
def localAddress: SocketAddress = self.localAddress
def remoteAddress: SocketAddress = self.remoteAddress
def peerCertificate: Option[Certificate] = self.peerCertificate
def close(deadline: Time): Future[Unit] = self.close(deadline)
def context: Context = self.context
override def toString: String = self.toString
Expand Down Expand Up @@ -412,7 +403,6 @@ class QueueTransport[In, Out](writeq: AsyncQueue[In], readq: AsyncQueue[Out])
val onClose: Future[Throwable] = closep
def localAddress: SocketAddress = context.localAddress
def remoteAddress: SocketAddress = context.remoteAddress
def peerCertificate: Option[Certificate] = context.peerCertificate
val context: TransportContext =
new SimpleTransportContext(new SocketAddress {}, new SocketAddress {}, None)
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ class TransportTest extends FunSuite with GeneratorDrivenPropertyChecks {
val onClose = new Promise[Throwable]
def localAddress = context.localAddress
def remoteAddress = context.remoteAddress
def peerCertificate = context.peerCertificate
def close(deadline: Time) = Future.exception(new Exception)
val context: TransportContext = new SimpleTransportContext()
}
Expand All @@ -118,7 +117,6 @@ class TransportTest extends FunSuite with GeneratorDrivenPropertyChecks {
val status = Status.Closed
def localAddress = context.localAddress
def remoteAddress = context.remoteAddress
def peerCertificate = context.peerCertificate
def close(deadline: Time) = Future.exception(new Exception)
val context: TransportContext = new SimpleTransportContext()
}
Expand Down Expand Up @@ -244,7 +242,6 @@ class TransportTest extends FunSuite with GeneratorDrivenPropertyChecks {
val onClose = Future.never
def localAddress = context.localAddress
def remoteAddress = context.remoteAddress
def peerCertificate = context.peerCertificate
def close(deadline: Time) = ???
val context: TransportContext = new SimpleTransportContext()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import com.twitter.finagle.Status
import com.twitter.finagle.transport.{SimpleTransportContext, Transport, TransportContext}
import com.twitter.util.{Future, Time}
import java.net.SocketAddress
import java.security.cert.Certificate

private[http2] final class DeadTransport(exn: Throwable, remote: SocketAddress)
extends Transport[Any, Any] {
Expand All @@ -17,7 +16,6 @@ private[http2] final class DeadTransport(exn: Throwable, remote: SocketAddress)
val context: TransportContext = new SimpleTransportContext(remoteAddress = remote)
def localAddress: SocketAddress = context.localAddress
def remoteAddress: SocketAddress = context.remoteAddress
def peerCertificate: Option[Certificate] = None

def write(req: Any): Future[Unit] = opsResult
def read(): Future[Any] = opsResult
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import com.twitter.util.{Future, Promise, Return, Throw, Time, Try}
import io.netty.handler.codec.http.{HttpObject, HttpRequest, LastHttpContent}
import io.netty.handler.codec.http2.Http2Error
import java.net.SocketAddress
import java.security.cert.Certificate
import java.util.concurrent.atomic.{AtomicBoolean, AtomicInteger}
import scala.collection.mutable.{HashMap => MutableHashMap}

Expand Down Expand Up @@ -561,8 +560,6 @@ final private[http2] class StreamTransportFactory(

def remoteAddress: SocketAddress = context.remoteAddress

def peerCertificate: Option[Certificate] = context.peerCertificate

def close(deadline: Time): Future[Unit] = {
wasClosed = true
exec.execute(new Runnable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.twitter.finagle.transport.Transport
import com.twitter.finagle.{ChannelClosedException, ChannelException, Status}
import com.twitter.util.{Future, Promise, Return, Time}
import java.net.SocketAddress
import java.security.cert.Certificate
import java.util.concurrent.atomic.AtomicBoolean
import org.jboss.netty.channel._

Expand Down Expand Up @@ -157,7 +156,6 @@ class ChannelTransport[In, Out](ch: Channel)

def localAddress: SocketAddress = context.localAddress
def remoteAddress: SocketAddress = context.remoteAddress
def peerCertificate: Option[Certificate] = context.peerCertificate

override def toString = s"Transport<channel=$ch, onClose=$closep>"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import com.twitter.finagle.transport.Transport
import com.twitter.util.{Future, Promise, Return, Time}
import io.netty.{channel => nettyChan}
import java.net.SocketAddress
import java.security.cert.Certificate
import java.util.concurrent.atomic.{AtomicBoolean, AtomicInteger}
import scala.util.control.NoStackTrace

Expand Down Expand Up @@ -146,7 +145,6 @@ private[finagle] class ChannelTransport(

def localAddress: SocketAddress = context.localAddress
def remoteAddress: SocketAddress = context.remoteAddress
def peerCertificate: Option[Certificate] = context.peerCertificate

override def toString = s"Transport<channel=$ch, onClose=${closed}>"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,11 @@ import com.twitter.finagle.transport.Transport
import com.twitter.util.{Await, Future, Return, Time, Throw}
import io.netty.channel.{ChannelException => _, _}
import io.netty.channel.embedded.EmbeddedChannel
import io.netty.handler.ssl.SslHandler
import org.scalatest.{FunSuite, OneInstancePerTest}
import org.scalatest.concurrent.Eventually._
import org.scalatest.mockito.MockitoSugar
import org.scalatest.prop.GeneratorDrivenPropertyChecks
import org.mockito.Mockito._
import java.security.cert.Certificate
import javax.net.ssl.{SSLEngine, SSLSession}

class ChannelTransportTest
extends FunSuite
Expand Down Expand Up @@ -154,18 +151,6 @@ class ChannelTransportTest
assert(transport.status == Status.Closed)
}

test("peerCertificate") {
val engine = mock[SSLEngine]
val session = mock[SSLSession]
val cert = mock[Certificate]
when(engine.getSession).thenReturn(session)
when(session.getPeerCertificates).thenReturn(Array(cert))
val ch = new EmbeddedChannel(new SslHandler(engine))
val tr = Transport.cast[String, String](new ChannelTransport(ch))

assert(tr.peerCertificate == Some(cert))
}

test("ChannelTransport drains the offer queue before reading from the channel") {
val channel = spy(new EmbeddedChannel())
channel.config().setAutoRead(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.twitter.finagle.transport.{Transport, TransportContext}
import com.twitter.io.Buf
import com.twitter.util.{Future, Time}
import java.net.SocketAddress
import java.security.cert.Certificate

/**
* A [[Transport]] implementation that uses [[StageDecoder]] to decode replies.
Expand Down Expand Up @@ -35,6 +34,5 @@ private[finagle] final class StageTransport(underlying: Transport[Buf, Buf])
override def onClose: Future[Throwable] = underlying.onClose
override def localAddress: SocketAddress = context.localAddress
override def remoteAddress: SocketAddress = context.remoteAddress
override def peerCertificate: Option[Certificate] = context.peerCertificate
val context: TransportContext = underlying.context
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import com.twitter.io.Buf
import com.twitter.logging.Level
import com.twitter.util.{Future, Try, Return, Promise, Throw, Updatable, Time}
import java.net.SocketAddress
import java.security.cert.Certificate
import java.util.logging.Logger
import org.apache.thrift.protocol.{TProtocolFactory, TMessage, TMessageType}
import scala.collection.mutable
Expand Down Expand Up @@ -48,7 +47,6 @@ private[finagle] object ThriftEmulator {
def onClose: Future[Throwable] = cur.onClose
def localAddress: SocketAddress = cur.localAddress
def remoteAddress: SocketAddress = cur.remoteAddress
def peerCertificate: Option[Certificate] = cur.peerCertificate
def close(deadline: Time): Future[Unit] = cur.close(deadline)
val context: UpdatableContext = new UpdatableContext(init.context)
override def toString: String = cur.toString
Expand Down

0 comments on commit ab0432b

Please sign in to comment.