Skip to content

Commit

Permalink
Issue #3916 - Improve multi range response Content-Length calc
Browse files Browse the repository at this point in the history 
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
@joakime
joakime committed Aug 8, 2020
1 parent 2206b3e commit dcf867c
Showing 1 changed file with 11 additions and 7 deletions.
18 changes: 11 additions & 7 deletions jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java
View file
Expand Up @@ -786,20 +786,24 @@ public String toString()
int length = 0;
String[] header = new String[ranges.size()];
int i = 0;
final int CRLF = "\r\n".length();
final int DASHDASH = "--".length();
final int BOUNDARY = multi.getBoundary().length();
final int FIELD_SEP = ": ".length();
for (InclusiveByteRange ibr : ranges)
{
header[i] = ibr.toHeaderRangeString(content_length);
if (i > 0) // in-part
length += 2;
length += 2 + multi.getBoundary().length() + 2; // "--" boundary CR LF
length += CRLF;
length += DASHDASH + BOUNDARY + CRLF;
if (mimetype != null)
length += HttpHeader.CONTENT_TYPE.asString().length() + 2 + mimetype.length() + 2; // "Content-Type" ": " <len> CR LF
length += HttpHeader.CONTENT_RANGE.asString().length() + 2 + header[i].length() + 2; // "Content-Range" ": " <len> CR LF
length += 2; // CR LF
length += ((ibr.getLast() - ibr.getFirst()) + 1); // content size
length += HttpHeader.CONTENT_TYPE.asString().length() + FIELD_SEP + mimetype.length() + CRLF;
length += HttpHeader.CONTENT_RANGE.asString().length() + FIELD_SEP + header[i].length() + CRLF;
length += CRLF;
length += ibr.getSize();
i++;
}
length += 2 + 2 + multi.getBoundary().length() + 2 + 2; // CR LF "--" boundary "--" CR LF
length += CRLF + DASHDASH + BOUNDARY + DASHDASH + CRLF;

This comment has been minimized.

Sign in to view

Copy link
@gregw

gregw Aug 9, 2020

Contributor

@joakime Perhaps move all these calculations to a static member on MultiPartOutputStream? That way the calculations will be nearer the code that writes the boundary and less likely to be forgotten if it is changed again?

This comment has been minimized.

Sign in to view

Copy link
@joakime

joakime Aug 9, 2020

Author Contributor

Been toying with String MutliPartOutputStream.genHeader(InclusiveByteRange ibr) as a possible solution as well.
Then we could just do a .length() on that return for the Content-Length calc, or use the String as-is for the actual output.
Though, I'm not 100% sold on this idea. So talk me out of it. ;-p

This comment has been minimized.

Sign in to view

Copy link
@gregw

gregw Aug 9, 2020

Contributor

I don't think that will work for large contents, as we would have to hold the whole string in memory! The writer abstraction allows the parts to be written part by part so we never hold the whole thing in memory. As a frequent users of ranges are streaming video clients, contents can be big.

This comment has been minimized.

Sign in to view

Copy link
@joakime

joakime Aug 9, 2020 •
edited

Author Contributor

I hear you, that's why I only want to generate the part header as a String, not the byte-range part or even the entire response. Just the individual part headers.

The above code would look like ...

int i=0;
int length = 0;
for (InclusiveByteRange ibr : ranges)
{
    if(i > 0) // in-part
        length += 2; // CRLF
    length += multi.genHeader(mimetype, ibr).length();
    length += ibr.size();
    i++;
}
length += multi.genClosingBoundary().length();
response.setContentLength(length);
response.setContentLength(length);

try (RangeWriter rangeWriter = HttpContentRangeWriter.newRangeWriter(content))
Expand Down

0 comments on commit dcf867c

Please sign in to comment.