Issue #2706 - Resource Service Incorrectly Returning 404

Flush response buffer in places where the response needs to be committed.
Removed if statement preventing HEAD requests processing conditional headers.
Added two new test cases which failed before the changes and should now pass.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2018-07-18 11:27:19 +10:00
parent c45ca9e38b
commit 6de77d26e2
3 changed files with 107 additions and 74 deletions

View File

@ -245,14 +245,14 @@ public class ResourceService
notFound(request,response);
return;
}
// Directory?
if (content.getResource().isDirectory())
{
sendWelcome(content,pathInContext,endsWithSlash,included,request,response);
return;
}
// Strip slash?
if (!included && endsWithSlash && pathInContext.length()>1)
{
@ -510,95 +510,95 @@ public class ResourceService
ifms=request.getHeader(HttpHeader.IF_MODIFIED_SINCE.asString());
ifums=request.getDateHeader(HttpHeader.IF_UNMODIFIED_SINCE.asString());
}
if (!HttpMethod.HEAD.is(request.getMethod()))
{
if (_etags)
{
String etag=content.getETagValue();
if (ifm!=null)
{
boolean match=false;
if (etag!=null)
{
QuotedCSV quoted = new QuotedCSV(true,ifm);
for (String tag : quoted)
{
if (CompressedContentFormat.tagEquals(etag, tag))
{
match=true;
break;
}
}
}
if (!match)
{
response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED);
return false;
}
}
if (ifnm!=null && etag!=null)
if (_etags)
{
String etag=content.getETagValue();
if (ifm!=null)
{
boolean match=false;
if (etag!=null)
{
// Handle special case of exact match OR gzip exact match
if (CompressedContentFormat.tagEquals(etag, ifnm) && ifnm.indexOf(',')<0)
{
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
response.setHeader(HttpHeader.ETAG.asString(),ifnm);
return false;
}
// Handle list of tags
QuotedCSV quoted = new QuotedCSV(true,ifnm);
QuotedCSV quoted = new QuotedCSV(true,ifm);
for (String tag : quoted)
{
if (CompressedContentFormat.tagEquals(etag, tag))
{
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
response.setHeader(HttpHeader.ETAG.asString(),tag);
return false;
match=true;
break;
}
}
// If etag requires content to be served, then do not check if-modified-since
return true;
}
if (!match)
{
response.setStatus(HttpServletResponse.SC_PRECONDITION_FAILED);
response.flushBuffer();
return false;
}
}
// Handle if modified since
if (ifms!=null)
if (ifnm!=null && etag!=null)
{
//Get jetty's Response impl
String mdlm=content.getLastModifiedValue();
if (mdlm!=null && ifms.equals(mdlm))
// Handle special case of exact match OR gzip exact match
if (CompressedContentFormat.tagEquals(etag, ifnm) && ifnm.indexOf(',')<0)
{
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
if (_etags)
response.setHeader(HttpHeader.ETAG.asString(),content.getETagValue());
response.setHeader(HttpHeader.ETAG.asString(),ifnm);
response.flushBuffer();
return false;
}
long ifmsl=request.getDateHeader(HttpHeader.IF_MODIFIED_SINCE.asString());
if (ifmsl!=-1 && content.getResource().lastModified()/1000 <= ifmsl/1000)
{
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
if (_etags)
response.setHeader(HttpHeader.ETAG.asString(),content.getETagValue());
response.flushBuffer();
return false;
// Handle list of tags
QuotedCSV quoted = new QuotedCSV(true,ifnm);
for (String tag : quoted)
{
if (CompressedContentFormat.tagEquals(etag, tag))
{
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
response.setHeader(HttpHeader.ETAG.asString(),tag);
response.flushBuffer();
return false;
}
}
// If etag requires content to be served, then do not check if-modified-since
return true;
}
}
// Parse the if[un]modified dates and compare to resource
if (ifums!=-1 && content.getResource().lastModified()/1000 > ifums/1000)
// Handle if modified since
if (ifms!=null)
{
//Get jetty's Response impl
String mdlm=content.getLastModifiedValue();
if (mdlm!=null && ifms.equals(mdlm))
{
response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED);
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
if (_etags)
response.setHeader(HttpHeader.ETAG.asString(),content.getETagValue());
response.flushBuffer();
return false;
}
long ifmsl=request.getDateHeader(HttpHeader.IF_MODIFIED_SINCE.asString());
if (ifmsl!=-1 && content.getResource().lastModified()/1000 <= ifmsl/1000)
{
response.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
if (_etags)
response.setHeader(HttpHeader.ETAG.asString(),content.getETagValue());
response.flushBuffer();
return false;
}
}
// Parse the if[un]modified dates and compare to resource
if (ifums!=-1 && content.getResource().lastModified()/1000 > ifums/1000)
{
response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED);
return false;
}
}
catch(IllegalArgumentException iae)
{
@ -606,6 +606,7 @@ public class ResourceService
response.sendError(400, iae.getMessage());
throw iae;
}
return true;
}
@ -649,7 +650,7 @@ public class ResourceService
final long content_length = content.getContentLengthValue();
// Get the output stream (or writer)
OutputStream out =null;
OutputStream out;
boolean written;
try
{
@ -687,6 +688,8 @@ public class ResourceService
BufferUtil.writeTo(buffer,out);
else
content.getResource().writeTo(out,0,content_length);
response.flushBuffer();
}
// else do a bypass write
else
@ -745,6 +748,7 @@ public class ResourceService
response.setHeader(HttpHeader.CONTENT_RANGE.asString(),
InclusiveByteRange.to416HeaderRangeString(content_length));
content.getResource().writeTo(out,0,content_length);
response.flushBuffer();
return true;
}
@ -761,6 +765,7 @@ public class ResourceService
response.setHeader(HttpHeader.CONTENT_RANGE.asString(),
singleSatisfiableRange.toHeaderRangeString(content_length));
content.getResource().writeTo(out,singleSatisfiableRange.getFirst(),singleLength);
response.flushBuffer();
return true;
}
@ -843,6 +848,7 @@ public class ResourceService
if (in!=null)
in.close();
multi.close();
response.flushBuffer();
}
return true;
}

View File

@ -269,14 +269,11 @@ public class ResourceHandler extends HandlerWrapper implements ResourceFactory,W
if (baseRequest.isHandled())
return;
if (!HttpMethod.GET.is(request.getMethod()))
if (!HttpMethod.GET.is(request.getMethod()) && !HttpMethod.HEAD.is(request.getMethod()))
{
if (!HttpMethod.HEAD.is(request.getMethod()))
{
// try another handler
super.handle(target,baseRequest,request,response);
return;
}
// try another handler
super.handle(target,baseRequest,request,response);
return;
}
_resourceService.doGet(request,response);

View File

@ -40,6 +40,7 @@ import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.HttpConfiguration;
@ -305,4 +306,33 @@ public class ResourceHandlerTest
}
}
@Test
public void testConditionalGetResponseCommitted() throws Exception
{
_config.setOutputBufferSize(8);
_resourceHandler.setEtags(true);
HttpTester.Response response = HttpTester.parseResponse(_local.getResponse("GET /resource/big.txt HTTP/1.0\r\n" +
"If-Match: \"NO_MATCH\"\r\n" +
"\r\n"));
assertThat(response.getStatus(),equalTo(HttpStatus.PRECONDITION_FAILED_412));
}
@Test
public void testConditionalHeadResponseCommitted() throws Exception
{
_config.setOutputBufferSize(8);
_resourceHandler.setEtags(true);
HttpTester.Response response = HttpTester.parseResponse(_local.getResponse("HEAD /resource/big.txt HTTP/1.0\r\n" +
"If-Match: \"NO_MATCH\"\r\n" +
"\r\n"));
assertThat(response.getStatus(),equalTo(HttpStatus.PRECONDITION_FAILED_412));
}
}