[PATCH] Support If-Modified-Since header on requests in httpd

classic Classic list List threaded Threaded
5 messages Options
jmp
Reply | Threaded
Open this post in threaded view
|

[PATCH] Support If-Modified-Since header on requests in httpd

jmp
If-Modified-Since is sent by http clients to be notified if a file has
been changed. This patch adds a function server_file_modified_since
that checks the time of the file from stat with the time sent from the
client. The separate function will help implement proper Range support.

I found 'timeoff' to be useful for converting to a time_t that is in
GMT; however, did not find documentation on this in the man pages. It
seems to be a function dating back to at least the NetBSD fork. If
there is a better time function I should be using please let me know.

The logic is separated out so we can reuse this in the future. I was
thinking this should be in http.c instead of server_file.c, but for
right now it is only useful for file operations. If-Modified-Since on
autoindex will not work due to how the index would be checked by this
code.

There is room for the 'If-Unmodified-Since' header, but it is not
really useful for file operations without Range support.


Index: usr.sbin/httpd/server_file.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
retrieving revision 1.51
diff -u -p -r1.51 server_file.c
--- usr.sbin/httpd/server_file.c 12 Feb 2015 10:05:29 -0000 1.51
+++ usr.sbin/httpd/server_file.c 18 Apr 2015 16:41:55 -0000
@@ -42,6 +42,7 @@ int server_file_request(struct httpd *,
     struct stat *);
 int server_file_index(struct httpd *, struct client *, struct stat *);
 int server_file_method(struct client *);
+int server_file_modified_since(struct http_descriptor *, struct stat *);
 
 int
 server_file_access(struct httpd *env, struct client *clt,
@@ -123,6 +124,10 @@ server_file_access(struct httpd *env, st
  goto fail;
  }
 
+ if ((ret = server_file_modified_since(desc, &st)) != -1) {
+ return ret;
+ }
+
  return (server_file_request(env, clt, path, &st));
 
  fail:
@@ -466,4 +471,24 @@ server_file_error(struct bufferevent *be
  }
  server_close(clt, "unknown event error");
  return;
+}
+
+int
+server_file_modified_since(struct http_descriptor * desc, struct stat * st)
+{
+ struct kv key, *since;
+ struct tm tm;
+
+ memset(&tm, 0, sizeof(struct tm));
+
+ key.kv_key = "If-Modified-Since";
+ if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
+    since->kv_value != NULL) {
+ if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) != NULL &&
+    timeoff(&tm, 0L) >= st->st_mtim.tv_sec) {
+ return 304;
+ }
+ }
+
+ return (-1);
 }

jmp
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Support If-Modified-Since header on requests in httpd

jmp
Sorry for the spam, I submitted the patch during the maintenance period. Any advice on this patch is appreciated.

Kyle Thompson

> On Apr 18, 2015, at 12:19 PM, jmp <[hidden email]> wrote:
>
> If-Modified-Since is sent by http clients to be notified if a file has
> been changed. This patch adds a function server_file_modified_since
> that checks the time of the file from stat with the time sent from the
> client. The separate function will help implement proper Range support.
>
> I found 'timeoff' to be useful for converting to a time_t that is in
> GMT; however, did not find documentation on this in the man pages. It
> seems to be a function dating back to at least the NetBSD fork. If
> there is a better time function I should be using please let me know.
>
> The logic is separated out so we can reuse this in the future. I was
> thinking this should be in http.c instead of server_file.c, but for
> right now it is only useful for file operations. If-Modified-Since on
> autoindex will not work due to how the index would be checked by this
> code.
>
> There is room for the 'If-Unmodified-Since' header, but it is not
> really useful for file operations without Range support.
>
>
> Index: usr.sbin/httpd/server_file.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 server_file.c
> --- usr.sbin/httpd/server_file.c    12 Feb 2015 10:05:29 -0000    1.51
> +++ usr.sbin/httpd/server_file.c    18 Apr 2015 16:41:55 -0000
> @@ -42,6 +42,7 @@ int     server_file_request(struct httpd *,
>        struct stat *);
> int     server_file_index(struct httpd *, struct client *, struct stat *);
> int     server_file_method(struct client *);
> +int     server_file_modified_since(struct http_descriptor *, struct stat *);
>
> int
> server_file_access(struct httpd *env, struct client *clt,
> @@ -123,6 +124,10 @@ server_file_access(struct httpd *env, st
>        goto fail;
>    }
>
> +    if ((ret = server_file_modified_since(desc, &st)) != -1) {
> +        return ret;
> +    }
> +
>    return (server_file_request(env, clt, path, &st));
>
>  fail:
> @@ -466,4 +471,24 @@ server_file_error(struct bufferevent *be
>    }
>    server_close(clt, "unknown event error");
>    return;
> +}
> +
> +int
> +server_file_modified_since(struct http_descriptor * desc, struct stat * st)
> +{
> +    struct kv         key, *since;
> +    struct tm         tm;
> +
> +    memset(&tm, 0, sizeof(struct tm));
> +
> +    key.kv_key = "If-Modified-Since";
> +    if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
> +        since->kv_value != NULL) {
> +        if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) != NULL &&
> +            timeoff(&tm, 0L) >= st->st_mtim.tv_sec) {
> +            return 304;
> +        }
> +    }
> +
> +    return (-1);
> }
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Support If-Modified-Since header on requests in httpd

Florian Obser-2
In reply to this post by jmp
On Sat, Apr 18, 2015 at 12:19:46PM -0500, jmp wrote:
> I found 'timeoff' to be useful for converting to a time_t that is in
> GMT; however, did not find documentation on this in the man pages. It
> seems to be a function dating back to at least the NetBSD fork. If
> there is a better time function I should be using please let me know.

I think timegm(3) is acceptable.

> Index: usr.sbin/httpd/server_file.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 server_file.c
> --- usr.sbin/httpd/server_file.c 12 Feb 2015 10:05:29 -0000 1.51
> +++ usr.sbin/httpd/server_file.c 18 Apr 2015 16:41:55 -0000
> @@ -42,6 +42,7 @@ int server_file_request(struct httpd *,
>      struct stat *);
>  int server_file_index(struct httpd *, struct client *, struct stat *);
>  int server_file_method(struct client *);
> +int server_file_modified_since(struct http_descriptor *, struct stat *);
>  
>  int
>  server_file_access(struct httpd *env, struct client *clt,
> @@ -123,6 +124,10 @@ server_file_access(struct httpd *env, st
>   goto fail;
>   }
>  
> + if ((ret = server_file_modified_since(desc, &st)) != -1) {
> + return ret;
> + }
> +

RFC 7232

   A recipient MUST ignore the If-Modified-Since header field if the
   received field-value is not a valid HTTP-date, or if the request
   method is neither GET nor HEAD.
             ^^^^^^^^^^^^^^^^^^^^

>   return (server_file_request(env, clt, path, &st));
>  
>   fail:
> @@ -466,4 +471,24 @@ server_file_error(struct bufferevent *be
>   }
>   server_close(clt, "unknown event error");
>   return;
> +}
> +
> +int
> +server_file_modified_since(struct http_descriptor * desc, struct stat * st)
> +{
> + struct kv key, *since;
> + struct tm tm;
> +
> + memset(&tm, 0, sizeof(struct tm));
> +
> + key.kv_key = "If-Modified-Since";
> + if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
> +    since->kv_value != NULL) {
> + if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) != NULL &&
> +    timeoff(&tm, 0L) >= st->st_mtim.tv_sec) {
> + return 304;
> + }
> + }

RFC 7231 defines 3 formats for HTTP-date and then goes on:
   A recipient that parses a timestamp value in an HTTP header field
   MUST accept all three HTTP-date formats.

I think it's ok here to only parse one variation and ignore
If-Modified-Since otherwise, we will just respond with a 200.

> +
> + return (-1);
>  }
>

Here is a tweaked version of the diff (check for GET/HEAD; timegm(3)).
This is OK florian@ or if someone wants to OK it I can commit it.


diff --git server_file.c server_file.c
index 3580bbb..9abf83b 100644
--- server_file.c
+++ server_file.c
@@ -42,6 +42,7 @@ int server_file_request(struct httpd *, struct client *, char *,
     struct stat *);
 int server_file_index(struct httpd *, struct client *, struct stat *);
 int server_file_method(struct client *);
+int server_file_modified_since(struct http_descriptor *, struct stat *);
 
 int
 server_file_access(struct httpd *env, struct client *clt,
@@ -123,6 +124,12 @@ server_file_access(struct httpd *env, struct client *clt,
  goto fail;
  }
 
+ if (desc->http_method == HTTP_METHOD_GET || desc->http_method ==
+    HTTP_METHOD_HEAD) {
+ if ((ret = server_file_modified_since(desc, &st)) != -1)
+ return ret;
+ }
+
  return (server_file_request(env, clt, path, &st));
 
  fail:
@@ -469,3 +476,23 @@ server_file_error(struct bufferevent *bev, short error, void *arg)
  server_close(clt, "unknown event error");
  return;
 }
+
+int
+server_file_modified_since(struct http_descriptor * desc, struct stat * st)
+{
+ struct kv key, *since;
+ struct tm tm;
+
+ memset(&tm, 0, sizeof(struct tm));
+
+ key.kv_key = "If-Modified-Since";
+ if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
+    since->kv_value != NULL) {
+ if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) !=
+    NULL && timegm(&tm) >= st->st_mtim.tv_sec) {
+ return (304);
+ }
+ }
+
+ return (-1);
+}


--
I'm not entirely sure you are real.

jmp
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Support If-Modified-Since header on requests in httpd

jmp
On Sun, May 03, 2015 at 03:00:40PM +0000, Florian Obser wrote:
> On Sat, Apr 18, 2015 at 12:19:46PM -0500, jmp wrote:
> RFC 7232
>
>    A recipient MUST ignore the If-Modified-Since header field if the
>    received field-value is not a valid HTTP-date, or if the request
>    method is neither GET nor HEAD.
>              ^^^^^^^^^^^^^^^^^^^^

Does httpd allow any other types of requests through server_file.c? All
other types of requests should only get sent through the CGI scripts. It
doesn't make since to allow POST, PUT, etc.. through to the file
handler.

>
> >   return (server_file_request(env, clt, path, &st));
> >  
> >   fail:
> > @@ -466,4 +471,24 @@ server_file_error(struct bufferevent *be
> >   }
> >   server_close(clt, "unknown event error");
> >   return;
> > +}
> > +
> > +int
> > +server_file_modified_since(struct http_descriptor * desc, struct stat * st)
> > +{
> > + struct kv key, *since;
> > + struct tm tm;
> > +
> > + memset(&tm, 0, sizeof(struct tm));
> > +
> > + key.kv_key = "If-Modified-Since";
> > + if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
> > +    since->kv_value != NULL) {
> > + if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) != NULL &&
> > +    timeoff(&tm, 0L) >= st->st_mtim.tv_sec) {
> > + return 304;
> > + }
> > + }
>
> RFC 7231 defines 3 formats for HTTP-date and then goes on:
>    A recipient that parses a timestamp value in an HTTP header field
>    MUST accept all three HTTP-date formats.
>
> I think it's ok here to only parse one variation and ignore
> If-Modified-Since otherwise, we will just respond with a 200.
>

From looking at Apache and nginx code, I wasn't able to see that they
used any other method. Like I said in my 'other' reply, we can always
extract this out to server_http since the Date header is created there.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Support If-Modified-Since header on requests in httpd

Florian Obser-2
On Sun, May 03, 2015 at 11:14:48AM -0500, Kyle Thompson wrote:

> On Sun, May 03, 2015 at 03:00:40PM +0000, Florian Obser wrote:
> > On Sat, Apr 18, 2015 at 12:19:46PM -0500, jmp wrote:
> > RFC 7232
> >
> >    A recipient MUST ignore the If-Modified-Since header field if the
> >    received field-value is not a valid HTTP-date, or if the request
> >    method is neither GET nor HEAD.
> >              ^^^^^^^^^^^^^^^^^^^^
>
> Does httpd allow any other types of requests through server_file.c? All
> other types of requests should only get sent through the CGI scripts. It
> doesn't make since to allow POST, PUT, etc.. through to the file
> handler.

yes, the check for that is later. This is better:

diff --git server_file.c server_file.c
index 3580bbb..42e2965 100644
--- server_file.c
+++ server_file.c
@@ -42,6 +42,7 @@ int server_file_request(struct httpd *, struct client *, char *,
     struct stat *);
 int server_file_index(struct httpd *, struct client *, struct stat *);
 int server_file_method(struct client *);
+int server_file_modified_since(struct http_descriptor *, struct stat *);
 
 int
 server_file_access(struct httpd *env, struct client *clt,
@@ -209,6 +210,9 @@ server_file_request(struct httpd *env, struct client *clt, char *path,
  goto abort;
  }
 
+ if ((ret = server_file_modified_since(clt->clt_descreq, st)) != -1)
+ return ret;
+
  /* Now open the file, should be readable or we have another problem */
  if ((fd = open(path, O_RDONLY)) == -1)
  goto abort;
@@ -469,3 +473,23 @@ server_file_error(struct bufferevent *bev, short error, void *arg)
  server_close(clt, "unknown event error");
  return;
 }
+
+int
+server_file_modified_since(struct http_descriptor * desc, struct stat * st)
+{
+ struct kv key, *since;
+ struct tm tm;
+
+ memset(&tm, 0, sizeof(struct tm));
+
+ key.kv_key = "If-Modified-Since";
+ if ((since = kv_find(&desc->http_headers, &key)) != NULL &&
+    since->kv_value != NULL) {
+ if (strptime(since->kv_value, "%a, %d %h %Y %T %Z", &tm) !=
+    NULL && timegm(&tm) >= st->st_mtim.tv_sec) {
+ return (304);
+ }
+ }
+
+ return (-1);
+}



--
I'm not entirely sure you are real.