LP-11: Cleaned up close handling

Addrecord · Mar 4, 2010 · e5b42c7 · e5b42c7
1 parent 9045fea
commit e5b42c7
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 42 deletions.
diff --git a/src/main/java/org/littleshoot/proxy/HttpRelayingHandler.java b/src/main/java/org/littleshoot/proxy/HttpRelayingHandler.java
@@ -2,6 +2,7 @@
 
 import org.jboss.netty.buffer.ChannelBuffers;
 import org.jboss.netty.channel.Channel;
+import org.jboss.netty.channel.ChannelFuture;
 import org.jboss.netty.channel.ChannelFutureListener;
 import org.jboss.netty.channel.ChannelHandlerContext;
 import org.jboss.netty.channel.ChannelPipelineCoverage;
@@ -161,8 +162,15 @@ public void channelOpen(final ChannelHandlerContext ctx,
     @Override
     public void channelClosed(final ChannelHandlerContext ctx, 
         final ChannelStateEvent e) throws Exception {
-        m_log.info("Got closed event on proxy -> web connection: "+e.getChannel());
-        //closeOnFlush(m_browserToProxyChannel);
+        m_log.info("Got closed event on proxy -> web connection: {}",
+            e.getChannel());
+
+        // This is vital this take place here and only here. If we handle this
+        // in other listeners, it's possible to get close events before
+        // we actually receive the HTTP response, in which case the response
+        // might never get back to the browser. It has to do with the order
+        // listeners are called in, but apparently the 
+        closeOnFlush(m_browserToProxyChannel);
     }
 
     @Override

diff --git a/src/main/java/org/littleshoot/proxy/HttpRequestHandler.java b/src/main/java/org/littleshoot/proxy/HttpRequestHandler.java
@@ -64,6 +64,8 @@ public class HttpRequestHandler extends SimpleChannelUpstreamHandler {
     private final ChannelGroup m_channelGroup;
     private final HttpRelayingHandlerFactory m_handlerFactory;
 
+    private volatile boolean m_closeOnResponse;
+
     /**
      * Creates a new class for handling HTTP requests with the specified
      * authentication manager.
@@ -107,14 +109,16 @@ private void processChunk(final ChannelHandlerContext ctx,
 
     private void processMessage(final ChannelHandlerContext ctx, 
         final MessageEvent me) {
-        final HttpRequest httpRequest = this.m_request = (HttpRequest) me.getMessage();
+        final HttpRequest httpRequest = 
+            this.m_request = (HttpRequest) me.getMessage();
 
         m_log.info("Got request: {} on channel: "+me.getChannel(), httpRequest);
         if (!this.m_authorizationManager.handleProxyAuthorization(httpRequest, ctx)) {
             return;
         }
         final String ae = httpRequest.getHeader(HttpHeaders.Names.ACCEPT_ENCODING);
         if (StringUtils.isNotBlank(ae)) {
+            // Remove sdch from encodings we accept since we can't decode it.
             final String noSdch = ae.replace(",sdch", "").replace("sdch", "");
             httpRequest.setHeader(HttpHeaders.Names.ACCEPT_ENCODING, noSdch);
             m_log.info("Removed sdch and inserted: {}", noSdch);
@@ -224,52 +228,27 @@ public void operationComplete(final ChannelFuture future)
                 }
             }
             else {
+                final ChannelFutureListener closedCfl = new ChannelFutureListener() {
+                    public void operationComplete(final ChannelFuture closed) 
+                        throws Exception {
+                        m_endpointsToChannelFutures.remove(m_hostAndPort);
+                    }
+                };
                 final ChannelFuture cf = 
                     newChannelFuture(httpRequestCopy, m_hostAndPort, inboundChannel);
                 final OnConnect onConnect = new OnConnect(cf);
                 m_endpointsToChannelFutures.put(m_hostAndPort, cf);
                 cf.addListener(new ChannelFutureListener() {
                     public void operationComplete(final ChannelFuture future)
                         throws Exception {
-                        m_channelGroup.add(future.getChannel());
+                        final Channel channel = future.getChannel();
+                        m_channelGroup.add(channel);
                         if (future.isSuccess()) {
                             m_log.info("Connected successfully to: {}", future.getChannel());
-                            final Channel newChannel = cf.getChannel();
-                            newChannel.getCloseFuture().addListener(
-                                new ChannelFutureListener() {
-                                public void operationComplete(
-                                    final ChannelFuture closeFuture)
-                                    throws Exception {
-                                    m_log.info("Got an outbound channel close event. Removing channel: "+newChannel);
-                                    m_log.info("Channel open??" +newChannel.isOpen());
-                                    m_endpointsToChannelFutures.remove(m_hostAndPort);
-                                    m_log.info("Outgoing channels on this connection: "+
-                                        m_endpointsToChannelFutures.size());
-                                    if (m_endpointsToChannelFutures.isEmpty()) {
-                                        m_log.info("All outbound channels closed...");
-
-                                        // We *don't* want to close here because
-                                        // the external site may have closed
-                                        // the connection due to a 
-                                        // Connection: close header, but we may
-                                        // not have actually processed the 
-                                        // response yet, and the client side
-                                        // is likely expecting more responses
-                                        // on this connection.
-                                        if (inboundChannel.isOpen()) {
-                                            m_log.info("Closing on flush...");
-                                            closeOnFlush(inboundChannel);
-                                        }
-                                    }
-                                    else {
-                                        m_log.info("Existing connections: {}", m_endpointsToChannelFutures);
-                                    }
-                                }
-                            });
+                            channel.getCloseFuture().addListener(closedCfl);
 
                             m_log.info("Writing message on channel...");
                             final ChannelFuture wf = onConnect.onConnect();
-                            //final ChannelFuture wf = newChannel.write(httpRequestCopy);
                             wf.addListener(new ChannelFutureListener() {
                                 public void operationComplete(final ChannelFuture wcf)
                                     throws Exception {
@@ -283,6 +262,7 @@ public void operationComplete(final ChannelFuture wcf)
                             if (m_totalInboundConnections == 1) {
                                 m_log.warn("Closing browser to proxy channel");
                                 me.getChannel().close();
+                                m_endpointsToChannelFutures.remove(m_hostAndPort);
                             }
                         }
                     }

diff --git a/src/main/java/org/littleshoot/proxy/HttpsServerPipelineFactory.java b/src/main/java/org/littleshoot/proxy/HttpsServerPipelineFactory.java
@@ -2,14 +2,9 @@
 
 import static org.jboss.netty.channel.Channels.pipeline;
 
-import javax.net.ssl.SSLEngine;
-
 import org.jboss.netty.channel.ChannelPipeline;
 import org.jboss.netty.channel.ChannelPipelineFactory;
 import org.jboss.netty.channel.group.ChannelGroup;
-import org.jboss.netty.handler.codec.http.HttpRequestDecoder;
-import org.jboss.netty.handler.codec.http.HttpResponseEncoder;
-import org.jboss.netty.handler.ssl.SslHandler;
 
 /**
  * Factory for creating pipelines for incoming requests to our listening