Skip to content

Commit

Permalink
Removed conditional compression - causing errors in Netty
Browse files Browse the repository at this point in the history 
HttpContentEncoder accedptedEncoding queue. When there is no
compression, the queue is not being cleared.
  • Loading branch information
@veebs
veebs committed Oct 5, 2011
1 parent 1f27f9c commit 3e92e3f
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 175 deletions.
20 changes: 0 additions & 20 deletions src/main/java/org/chililog/server/workbench/ApiRequestHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import org.jboss.netty.buffer.ChannelBuffers;
import org.jboss.netty.channel.ChannelFuture;
import org.jboss.netty.channel.ChannelFutureListener;
import org.jboss.netty.channel.ChannelHandler;
import org.jboss.netty.channel.ChannelHandlerContext;
import org.jboss.netty.channel.MessageEvent;
import org.jboss.netty.handler.codec.http.DefaultHttpResponse;
Expand Down Expand Up @@ -330,7 +329,6 @@ private void writeResponse(ChannelHandlerContext ctx, MessageEvent e, ApiResult
response.setHeader(CONTENT_TYPE, result.getResponseContentType());
if (result.getResponseContentIOStyle() == ContentIOStyle.ByteArray) {
byte[] content = (byte[]) result.getResponseContent();
toogleCompression(ctx, content.length > 4096); // Compress if > 4K
response.setContent(ChannelBuffers.copiedBuffer(content));
} else {
throw new NotImplementedException("ContentIOStyle " + result.getResponseContentIOStyle().toString());
Expand Down Expand Up @@ -368,9 +366,6 @@ private void writeResponse(ChannelHandlerContext ctx, MessageEvent e, Exception
// Decide whether to close the connection or not.
boolean keepAlive = isKeepAlive(_request);

// No need to compress errors. Will be small in size
toogleCompression(ctx, false);

// Build the response object.
HttpResponse response = new DefaultHttpResponse(HTTP_1_1, HttpResponseStatus.INTERNAL_SERVER_ERROR);
setDateHeader(response);
Expand Down Expand Up @@ -445,21 +440,6 @@ private void setDateHeader(HttpResponse response) {
response.setHeader(HttpHeaders.Names.DATE, dateFormatter.format(time.getTime()));
}

/**
* Turn on/off compression
*
* @param ctx
* context
* @param doCompression
* True to turn compression on, False to turn it off
*/
private void toogleCompression(ChannelHandlerContext ctx, boolean doCompression) {
ChannelHandler deflater = ctx.getPipeline().get("deflater");
if (deflater instanceof ConditionalHttpContentCompressor) {
((ConditionalHttpContentCompressor) deflater).setDoCompression(doCompression);
}
}

/**
* Write a log entry about the request
*
Expand Down
93 changes: 0 additions & 93 deletions src/main/java/org/chililog/server/workbench/ConditionalHttpContentCompressor.java
View file

This file was deleted.

3 changes: 2 additions & 1 deletion src/main/java/org/chililog/server/workbench/HttpServerPipelineFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.chililog.server.common.AppProperties;
import org.jboss.netty.channel.ChannelPipeline;
import org.jboss.netty.channel.ChannelPipelineFactory;
import org.jboss.netty.handler.codec.http.HttpContentCompressor;
import org.jboss.netty.handler.codec.http.HttpRequestDecoder;
import org.jboss.netty.handler.codec.http.HttpResponseEncoder;
import org.jboss.netty.handler.ssl.SslHandler;
Expand Down Expand Up @@ -75,7 +76,7 @@ public ChannelPipeline getPipeline() throws Exception {
}

// Compress
pipeline.addLast("deflater", new ConditionalHttpContentCompressor());
pipeline.addLast("deflater", new HttpContentCompressor(1));

// Handler to dispatch processing to our services
pipeline.addLast("handler", new HttpRequestHandler());
Expand Down
76 changes: 16 additions & 60 deletions src/main/java/org/chililog/server/workbench/StaticFileRequestHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.Calendar;
import java.util.Date;
import java.util.GregorianCalendar;
import java.util.List;
import java.util.Locale;
import java.util.TimeZone;

Expand All @@ -44,7 +45,6 @@
import org.jboss.netty.channel.Channel;
import org.jboss.netty.channel.ChannelFuture;
import org.jboss.netty.channel.ChannelFutureListener;
import org.jboss.netty.channel.ChannelHandler;
import org.jboss.netty.channel.ChannelHandlerContext;
import org.jboss.netty.channel.MessageEvent;
import org.jboss.netty.handler.codec.http.DefaultHttpResponse;
Expand Down Expand Up @@ -165,10 +165,6 @@ public void processMessage(ChannelHandlerContext ctx, MessageEvent e) throws Exc
// Log
writeLogEntry(e, OK, null);

// Turn compression on/off
boolean doCompression = checkDoCompression(filePath, fileLength);
toogleCompression(ctx, doCompression);

// Create the response
HttpResponse response = new DefaultHttpResponse(HTTP_1_1, OK);
setContentLength(response, fileLength);
Expand All @@ -178,16 +174,7 @@ public void processMessage(ChannelHandlerContext ctx, MessageEvent e) throws Exc
// Write the content.
Channel ch = e.getChannel();
ChannelFuture writeFuture;
if (doCompression) {
// Cannot use ChunkedFile or zero-copy if we want to do compression
// Must read file contents and set it as the contents
byte[] buffer = new byte[(int) fileLength];
raf.readFully(buffer);
raf.close();

response.setContent(ChannelBuffers.copiedBuffer(buffer));
writeFuture = ch.write(response);
} else if (AppProperties.getInstance().getWorkbenchSslEnabled()) {
if (AppProperties.getInstance().getWorkbenchSslEnabled()) {
// Cannot use zero-copy with HTTPS

// Write the initial line and the header.
Expand Down Expand Up @@ -283,17 +270,19 @@ private String convertUriToPhysicalFilePath(String uri) throws UnsupportedEncodi
private void sendError(ChannelHandlerContext ctx, MessageEvent e, HttpResponseStatus status, String moreInfo) {
writeLogEntry(e, status, moreInfo);

toogleCompression(ctx, false);

HttpResponse response = new DefaultHttpResponse(HTTP_1_1, status);
setDateHeader(response);

// Send error back as plain text in the body
response.setHeader(CONTENT_TYPE, "text/plain; charset=UTF-8");
response.setContent(ChannelBuffers.copiedBuffer("Failure: " + status.toString() + "\r\n", CharsetUtil.UTF_8));

// Close the connection as soon as the error message is sent.
ctx.getChannel().write(response).addListener(ChannelFutureListener.CLOSE);
ChannelFuture writeFuture = ctx.getChannel().write(response);

// Decide whether to close the connection or not.
if (!isKeepAlive((HttpRequest)e.getMessage())) {
writeFuture.addListener(ChannelFutureListener.CLOSE);
}
}

/**
Expand All @@ -307,12 +296,9 @@ private void sendError(ChannelHandlerContext ctx, MessageEvent e, HttpResponseSt
private void sendNotModified(ChannelHandlerContext ctx, MessageEvent e) {
writeLogEntry(e, HttpResponseStatus.NOT_MODIFIED, null);

toogleCompression(ctx, false);

HttpResponse response = new DefaultHttpResponse(HTTP_1_1, HttpResponseStatus.NOT_MODIFIED);
setDateHeader(response);

// Close the connection as soon as the error message is sent.
ChannelFuture writeFuture = ctx.getChannel().write(response);

// Decide whether to close the connection or not.
Expand All @@ -321,30 +307,6 @@ private void sendNotModified(ChannelHandlerContext ctx, MessageEvent e) {
}
}

/**
* <p>
* Figure out if we should do try to do HTTP compression or not based on the file extension and file size
* </p>
*
* @param filePath
* Path to the file
* @return true if compression on the file should be performed, false if not
*/
private boolean checkDoCompression(String filePath, long fileLength) {
// If < 4096 bytes, compression makes the file bigger and/or CPU used vs time saved is small
// If > 1 MB, don't compress. Just chunk download because it takes too much memory to read everything in
if (fileLength < 4096 || fileLength > 1048576) {
return false;
}

String s = filePath.toLowerCase();
if (s.endsWith(".html") || s.endsWith(".js") || s.endsWith(".css") || s.endsWith(".txt") || s.endsWith(".json")
|| s.endsWith(".xml")) {
return true;
}
return false;
}

/**
* Sets the content type header for the HTTP Response
*
Expand Down Expand Up @@ -431,20 +393,6 @@ private void setDateAndCacheHeaders(HttpResponse response, File filetoCache) {
response.setHeader(HttpHeaders.Names.LAST_MODIFIED, dateFormatter.format(new Date(filetoCache.lastModified())));
}

/**
* Turn on/off compression
*
* @param ctx
* context
* @param doCompression
* True to turn compression on, False to turn it off
*/
private void toogleCompression(ChannelHandlerContext ctx, boolean doCompression) {
ChannelHandler deflater = ctx.getPipeline().get("deflater");
if (deflater instanceof ConditionalHttpContentCompressor) {
((ConditionalHttpContentCompressor) deflater).setDoCompression(doCompression);
}
}

/**
* Write audit log entry
Expand All @@ -456,6 +404,14 @@ private void writeLogEntry(MessageEvent e, HttpResponseStatus status, String mor
HttpRequest request = (HttpRequest) e.getMessage();
_logger.info("GET %s REMOTE_IP=%s STATUS=%s CHANNEL=%s %s", request.getUri(), e.getRemoteAddress().toString(),
status, e.getChannel().getId(), moreInfo == null ? StringUtils.EMPTY : moreInfo);

StringBuilder sb = new StringBuilder("Request Headers\n");
List<java.util.Map.Entry<String, String>> headers = request.getHeaders();
for (java.util.Map.Entry<String, String> entry : headers){
sb.append(entry.getKey()).append("=").append(entry.getValue()).append("\n");
}
_logger.debug(sb.toString());

return;
}
}
2 changes: 1 addition & 1 deletion src/test/java/org/chililog/server/workbench/WorkbenchServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ public void testStaticFileCompression() throws IOException, ParseException, Deco
}
}

// Should get back a 304
// Should get back a 200
assertEquals("HTTP/1.1 200 OK", responseCode);
assertTrue(!StringUtils.isBlank(headers.get("Date")));

Expand Down

0 comments on commit 3e92e3f

Please sign in to comment.