On 6/28/20 1:41 AM, minf...@apache.org wrote:
> Author: minfrin
> Date: Sat Jun 27 23:41:00 2020
> New Revision: 1879285
>
> URL: http://svn.apache.org/viewvc?rev=1879285&view=rev
> Log:
> "[mod_dav_fs etag handling] should really honor the FileETag setting".
> - It now does.
> - Add "Digest" to FileETag directive, allowing a strong ETag to be
> generated using a file digest.
> - Add ap_make_etag_ex() and ap_set_etag_fd() to allow full control over
> ETag generation.
> - Add concept of "binary notes" to request_rec, allowing packed bit flags
> to be added to a request.
> - First binary note - AP_REQUEST_STRONG_ETAG - allows modules to force
> the ETag to a strong ETag to comply with RFC requirements, such as those
> mandated by various WebDAV extensions.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/manual/mod/core.xml
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/http_core.h
> httpd/httpd/trunk/include/http_protocol.h
> httpd/httpd/trunk/include/httpd.h
> httpd/httpd/trunk/modules/dav/fs/repos.c
> httpd/httpd/trunk/modules/test/mod_dialup.c
> httpd/httpd/trunk/server/core.c
> httpd/httpd/trunk/server/util_etag.c
>
> Modified: httpd/httpd/trunk/server/util_etag.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_etag.c?rev=1879285&r1=1879284&r2=1879285&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/util_etag.c (original)
> +++ httpd/httpd/trunk/server/util_etag.c Sat Jun 27 23:41:00 2020
> @@ -73,13 +99,96 @@ AP_DECLARE(char *) ap_make_etag(request_
> cfg = (core_dir_config *)ap_get_core_module_config(r->per_dir_config);
> etag_bits = (cfg->etag_bits & (~ cfg->etag_remove)) | cfg->etag_add;
>
> + if (er->force_weak) {
> + weak = ETAG_WEAK;
> + weak_len = sizeof(ETAG_WEAK);
> + }
> +
> + if (r->vlist_validator) {
> +
> + /* If we have a variant list validator (vlv) due to the
> + * response being negotiated, then we create a structured
> + * entity tag which merges the variant etag with the variant
> + * list validator (vlv). This merging makes revalidation
> + * somewhat safer, ensures that caches which can deal with
> + * Vary will (eventually) be updated if the set of variants is
> + * changed, and is also a protocol requirement for transparent
> + * content negotiation.
> + */
> +
> + /* if the variant list validator is weak, we make the whole
> + * structured etag weak. If we would not, then clients could
> + * have problems merging range responses if we have different
> + * variants with the same non-globally-unique strong etag.
> + */
> +
> + vlv = r->vlist_validator;
> + if (vlv[0] == 'W') {
> + vlv += 3;
> + weak = ETAG_WEAK;
> + weak_len = sizeof(ETAG_WEAK);
> + }
> + else {
> + vlv++;
> + }
> + vlv_len = strlen(vlv);
> +
> + }
> + else {
> + vlv = NULL;
> + vlv_len = 0;
> + }
> +
> + /*
> + * Did a module flag the need for a strong etag, or did the
> + * configuration tell us to generate a digest?
> + */
> + if (er->finfo->filetype == APR_REG &&
> + (AP_REQUEST_IS_STRONG_ETAG(r) || (etag_bits & ETAG_DIGEST))) {
> +
> + apr_sha1_ctx_t context;
> + unsigned char buf[4096]; /* keep this a multiple of 64 */
> + unsigned char digest[APR_SHA1_DIGESTSIZE];
> + apr_file_t *fd = NULL;
> +
> + apr_size_t nbytes;
> + apr_off_t offset = 0L;
> +
> + if (er->fd) {
> + fd = er->fd;
> + }
> + else if (er->pathname) {
> + apr_file_open(&fd, er->pathname, APR_READ | APR_BINARY, 0,
> r->pool);
> + }
> + if (!fd) {
> + return "";
> + }
> +
> + etag = apr_palloc(r->pool, weak_len + sizeof("\"\"") +
> + 4*(APR_SHA1_DIGESTSIZE/3) + vlv_len + 4);
Using apr_base64_encode_len in the formula above would make it easier to
understand and read IMHO.
> +
> + apr_sha1_init(&context);
> + nbytes = sizeof(buf);
> + while (apr_file_read(fd, buf, &nbytes) == APR_SUCCESS) {
I assume that the code has been taken from ap_md5digest, but
if we have MMAP available and if we have not disabled MMAP we use MMAP to read
the contents of the file
if sendfile is not possible (e.g. SSL connections).
Wouldn't using MMAP more performant here especially in case of larger files?
> + apr_sha1_update_binary(&context, buf, nbytes);
> + nbytes = sizeof(buf);
> + }
> + apr_file_seek(fd, APR_SET, &offset);
Why do we always reset the file pointer to 0? Why don't we get the actual file
pointer of fd before we do the reading
and restore it afterwards?
> + apr_sha1_final(digest, &context> +
> + etag_start(etag, weak, &next);
> + next += apr_base64_encode_binary(next, digest, APR_SHA1_DIGESTSIZE)
> - 1;
> + etag_end(next, vlv, vlv_len);
> +
> + return etag;
> + }
> +
> /*
> * If it's a file (or we wouldn't be here) and no ETags
> * should be set for files, return an empty string and
> * note it for the header-sender to ignore.
> */
> if (etag_bits & ETAG_NONE) {
> - apr_table_setn(r->notes, "no-etag", "omit");
> return "";
> }
>
Regards
Rüdiger
