It doesn't make sense for the client to ask for a certain byte range
unless it already has the other bytes. It needs to make sure that
it's not asking for the tail of a file which has changed in the
server.
The code doesn't handle If-Range. If-Modified-Since is handled in the
existing code and returns early before the range code runs. I don't
see how this range implementation could work in the real world.
There's no tests for any of this.
Maybe a better way to handle multiple ranges is by using an iovec
struct. We can pass the result directly to Ns_ConnSend() or something
similar.
The range code only handles fastpath requests, but it should probably
also handle stuff like ns_returnfp/ns_respond. I don't see any reason
why it shouldn't also handle the fastpath cache.
The handling of response codes looks a little hairy. How about if
ParseRange just returned the correct response code? All error
checking should be moved inside that function. At the moment
ParseRange handles some errors, the calling code others, which is
tricky to follow. Also, it's extremely difficult to figure out what
the parsing code is doing.
AOLserver 4.5 has support for chunked responses. We should probably
check to see how that might impact things.
I'm wondering if the best way forward is to back out the range code,
make a quick release, then add it back for the next one along with the
TclVFS stuff, file upload improvements, and hopefully my mythical
caching changes, all of which are pretty invasive and potentially
destabilizing. What does everyone think?
On 6/29/05, Vlad Seryakov <[EMAIL PROTECTED]> wrote:
> Update of /cvsroot/naviserver/naviserver/nsd
> In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv4535/nsd
>
> Modified Files:
> fastpath.c
> Log Message:
> see ChangeLog
>
>
> Index: fastpath.c
> ===================================================================
> RCS file: /cvsroot/naviserver/naviserver/nsd/fastpath.c,v
> retrieving revision 1.9
> retrieving revision 1.10
> diff -C2 -d -r1.9 -r1.10
> *** fastpath.c 29 Jun 2005 23:27:50 -0000 1.9
> --- fastpath.c 30 Jun 2005 01:14:02 -0000 1.10
> ***************
> *** 39,42 ****
> --- 39,44 ----
> NS_RCSID("@(#) $Header$");
>
> + #define MAX_RANGES 10
> +
> /*
> * The following structure defines the contents of a file
> ***************
> *** 61,66 ****
> static int FastReturn(NsServer *servPtr, Ns_Conn *conn, int status,
> char *type, char *file, struct stat *stPtr);
> ! static int ParseRange(Ns_Conn *conn, unsigned long size,
> ! unsigned long *offset1, unsigned long *offset2);
>
> /*
> --- 63,68 ----
> static int FastReturn(NsServer *servPtr, Ns_Conn *conn, int status,
> char *type, char *file, struct stat *stPtr);
> ! static int ParseRange(Ns_Conn *conn, unsigned long size, unsigned long
> *offsets,
> ! int offsets_size);
>
> /*
> ***************
> *** 474,479 ****
> {
> int fd, new, nread;
> ! unsigned long size, offset1, offset2;
> ! int result = NS_ERROR, range = NS_ERROR;
> char *key;
> Ns_Entry *entPtr;
> --- 476,481 ----
> {
> int fd, new, nread;
> ! unsigned long size, offsets[MAX_RANGES*2];
> ! int result = NS_ERROR, ranges = 0;
> char *key;
> Ns_Entry *entPtr;
> ***************
> *** 520,537 ****
> size = stPtr->st_size;
> if (status == 200) {
> ! range = ParseRange(conn, stPtr->st_size, &offset1, &offset2);
> }
> ! if (range != NS_ERROR) {
> ! if (offset1 > offset2) {
> ! /* 416 Requested Range Not Satisfiable */
> ! Ns_ConnPrintfHeaders(conn, "Content-Range", "bytes */%lu",
> ! stPtr->st_size);
> ! Ns_ConnSetRequiredHeaders(conn, type, (int) stPtr->st_size);
> ! return Ns_ConnFlushHeaders(conn, 416);
> ! }
> /* Continue with returning a portion of the file */
> Ns_ConnPrintfHeaders(conn, "Content-Range", "bytes %lu-%lu/%lu",
> ! offset1, offset2, stPtr->st_size);
> ! size = (offset2 - offset1) + 1;
> /* 206 Partial Content */
> status = 206;
> --- 522,539 ----
> size = stPtr->st_size;
> if (status == 200) {
> ! ranges = ParseRange(conn, stPtr->st_size, offsets, MAX_RANGES*2);
> }
> ! if (ranges == -1) {
> ! /* 416 Requested Range Not Satisfiable */
> ! Ns_ConnPrintfHeaders(conn, "Content-Range", "bytes */%lu",
> ! stPtr->st_size);
> ! Ns_ConnSetRequiredHeaders(conn, type, (int) stPtr->st_size);
> ! return Ns_ConnFlushHeaders(conn, 416);
> ! }
> ! if (ranges == 1) {
> /* Continue with returning a portion of the file */
> Ns_ConnPrintfHeaders(conn, "Content-Range", "bytes %lu-%lu/%lu",
> ! offsets[0], offsets[1], stPtr->st_size);
> ! size = (offsets[1] - offsets[0]) + 1;
> /* 206 Partial Content */
> status = 206;
> ***************
> *** 540,544 ****
> if (servPtr->fastpath.cache == NULL
> || stPtr->st_size > servPtr->fastpath.cachemaxentry
> ! || range == NS_OK) {
>
> /*
> --- 542,546 ----
> if (servPtr->fastpath.cache == NULL
> || stPtr->st_size > servPtr->fastpath.cachemaxentry
> ! || ranges == 1) {
>
> /*
> ***************
> *** 552,557 ****
> && NsMemMap(file, stPtr->st_size, NS_MMAP_READ, &fmap) ==
> NS_OK) {
> char *maddr = fmap.addr;
> ! if (range == NS_OK) {
> ! maddr += offset1;
> }
> result = Ns_ConnReturnData(conn,status, maddr, size, type);
> --- 554,559 ----
> && NsMemMap(file, stPtr->st_size, NS_MMAP_READ, &fmap) ==
> NS_OK) {
> char *maddr = fmap.addr;
> ! if (ranges > 0) {
> ! maddr += offsets[0];
> }
> result = Ns_ConnReturnData(conn,status, maddr, size, type);
> ***************
> *** 564,569 ****
> goto notfound;
> }
> ! if (range == NS_OK) {
> ! lseek(fd, offset1, SEEK_SET);
> }
> result = Ns_ConnReturnOpenFd(conn, status, type, fd, size);
> --- 566,571 ----
> goto notfound;
> }
> ! if (ranges > 0) {
> ! lseek(fd, offsets[0], SEEK_SET);
> }
> result = Ns_ConnReturnOpenFd(conn, status, type, fd, size);
> ***************
> *** 690,694 ****
> *
> * Results:
> ! * NS_OK or NS_ERROR
> *
> * Side effects:
> --- 692,696 ----
> *
> * Results:
> ! * -1 on error, number of byte ranges parsed
> *
> * Side effects:
> ***************
> *** 699,755 ****
>
> static int
> ! ParseRange(Ns_Conn *conn, unsigned long size,
> ! unsigned long *offset1, unsigned long *offset2)
> {
> char *range;
>
> ! *offset1 = *offset2 = 0;
>
> ! if ((range = Ns_SetIGet(conn->headers, "Range")) != NULL
> ! && (range = strstr(range,"bytes=")) != NULL) {
> ! range += 6;
> ! /* Multiple ranges are not supported yet */
> ! if (strchr(range,',') != NULL) {
> ! return NS_ERROR;
> ! }
> if (isdigit(*range)) {
> ! *offset1 = atol(range);
> while (isdigit(*range)) range++;
> if (*range == '-') {
> range++;
> if (!isdigit(*range)) {
> ! if (*range != '\0') {
> ! return NS_ERROR;
> ! }
> ! *offset2 = size - 1;
> } else {
> ! *offset2 = atol(range);
> ! if (*offset1 > *offset2) {
> ! return NS_ERROR;
> }
> ! if (*offset2 >= size) {
> ! *offset2 = size - 1;
> }
> }
> ! return NS_OK;
> ! } else {
> ! return NS_ERROR;
> }
> } else if (*range == '-') {
> range++;
> if (!isdigit(*range)) {
> ! return NS_ERROR;
> }
> ! *offset2 = atol(range);
> ! if (*offset2 > size) {
> ! *offset2 = size;
> }
> /* Size from the end requested, convert into offset */
> ! *offset1 = size - *offset2;
> ! *offset2 = *offset1 + *offset2 - 1;
> ! return NS_OK;
> }
> }
> ! return NS_ERROR;
> }
>
> --- 701,780 ----
>
> static int
> ! ParseRange(Ns_Conn *conn, unsigned long size, unsigned long *offsets,
> ! int offsets_size)
> {
> + int count = 0;
> char *range;
>
> ! if ((range = Ns_SetIGet(conn->headers, "Range")) == NULL
> ! || (range = strstr(range,"bytes=")) == NULL) {
> ! return 0;
> ! }
> ! range += 6;
> ! memset(offsets,0,sizeof(unsigned long)*offsets_size);
>
> ! while(*range && count < offsets_size-1) {
> if (isdigit(*range)) {
> ! offsets[count] = atol(range);
> while (isdigit(*range)) range++;
> if (*range == '-') {
> range++;
> if (!isdigit(*range)) {
> ! offsets[count+1] = size - 1;
> } else {
> ! offsets[count+1] = atol(range);
> ! if (offsets[count] > offsets[count+1]) {
> ! return 0;
> }
> ! if (offsets[count+1] >= size) {
> ! offsets[count+1] = size - 1;
> }
> + while (isdigit(*range)) range++;
> }
> ! switch (*range) {
> ! case ',':
> ! range++;
> ! case '\0':
> ! break;
> ! default:
> ! return 0;
> ! }
> ! if (offsets[count] > offsets[count+1]) {
> ! return -1;
> ! }
> ! count += 2;
> ! continue;
> }
> } else if (*range == '-') {
> range++;
> if (!isdigit(*range)) {
> ! return 0;
> }
> ! offsets[count+1] = atol(range);
> ! if (offsets[count+1] > size) {
> ! offsets[count+1] = size;
> }
> /* Size from the end requested, convert into offset */
> ! offsets[count] = size - offsets[count+1];
> ! offsets[count+1] = offsets[count] + offsets[count+1] - 1;
> ! while (isdigit(*range)) range++;
> ! switch (*range) {
> ! case ',':
> ! range++;
> ! case '\0':
> ! break;
> ! default:
> ! return 0;
> ! }
> ! if (offsets[count] > offsets[count+1]) {
> ! return -1;
> ! }
> ! count += 2;
> ! continue;
> }
> + /* Invalid syntax */
> + return 0;
> }
> ! return count/2;
> }
