Passed: apache/httpd#905 (trunk - c17967b)

[Travis CI]

Build Update for apache/httpd
-

Build: #905
Status: Passed

Duration: 20 mins and 21 secs
Commit: c17967b (trunk)
Author: Luca Toscano
Message: server/util_script.c: reserve one APLOGNO number after r1879253




git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1879348 
13f79535-47bb-0310-9956-ffa450edef68

View the changeset: 
https://github.com/apache/httpd/compare/e8417d72a296...c17967b44758

View the full build log and details: 
https://travis-ci.org/github/apache/httpd/builds/703278564?utm_medium=notification&utm_source=email


--

You can unsubscribe from build emails from the apache/httpd repository going to 
https://travis-ci.org/account/preferences/unsubscribe?repository=69847&utm_medium=notification&utm_source=email.
Or unsubscribe from *all* email updating your settings at 
https://travis-ci.org/account/preferences/unsubscribe?utm_medium=notification&utm_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.

Passed: apache/httpd#904 (trunk - e8417d7)

[Travis CI]

Build Update for apache/httpd
-

Build: #904
Status: Passed

Duration: 2 mins and 46 secs
Commit: e8417d7 (trunk)
Author: Graham Leggett
Message: Use a dedicated constant for the base64 sha1 length.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1879346 
13f79535-47bb-0310-9956-ffa450edef68

View the changeset: 
https://github.com/apache/httpd/compare/69701ffdadce...e8417d72a296

View the full build log and details: 
https://travis-ci.org/github/apache/httpd/builds/703245195?utm_medium=notification&utm_source=email


--

You can unsubscribe from build emails from the apache/httpd repository going to 
https://travis-ci.org/account/preferences/unsubscribe?repository=69847&utm_medium=notification&utm_source=email.
Or unsubscribe from *all* email updating your settings at 
https://travis-ci.org/account/preferences/unsubscribe?utm_medium=notification&utm_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.

Re: svn commit: r1879253 - /httpd/httpd/trunk/server/util_script.c

[Luca Toscano]

On Mon, Jun 29, 2020 at 9:12 AM Ruediger Pluem  wrote:
>
>
>
> On 6/27/20 11:11 AM, elu...@apache.org wrote:
> > Author: elukey
> > Date: Sat Jun 27 09:11:32 2020
> > New Revision: 1879253
> >
> > URL: http://svn.apache.org/viewvc?rev=1879253&view=rev
> > Log:
> > server/util_script.c: tune logging Last-Modified header
> >
> > Follow up after Joe's feedback in STATUS:
> >   - If APR_DATE_BAD is returned for Last-Modified, log it at INFO level
> > (as opposed to trace).
> >   - Remove unnecessary guard for APLOGrtrace1(r).
>
>
> As this is now a APLOG_INFO, can you please add an APLOGNO?
> For further details please have a look at docs/log-message-tags/README

Thanks for the pointer, hopefully fixed with r1879348.

Luca

Re: svn commit: r1879346 - /httpd/httpd/trunk/server/util_etag.c

[Ruediger Pluem]

On 6/29/20 7:45 PM, minf...@apache.org wrote:
> Author: minfrin
> Date: Mon Jun 29 17:45:35 2020
> New Revision: 1879346
> 
> URL: http://svn.apache.org/viewvc?rev=1879346&view=rev
> Log:
> Use a dedicated constant for the base64 sha1 length.
> 
> Modified:
> 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=1879346&r1=1879345&r2=1879346&view=diff
> ==
> --- httpd/httpd/trunk/server/util_etag.c (original)
> +++ httpd/httpd/trunk/server/util_etag.c Mon Jun 29 17:45:35 2020
> @@ -29,6 +29,8 @@
>  #include "http_protocol.h"   /* For index_of_response().  Grump. */
>  #include "http_request.h"
>  
> +#define SHA1_DIGEST_BASE64_LEN 4*(APR_SHA1_DIGESTSIZE/3)

Thanks for the better readability, but using apr_base64_encode_len this would 
result in

(APR_SHA1_DIGESTSIZE + 2) / 3 * 4) + 1

Why is the calculation different here (Ok I guess '+ 1' in the above is for the 
final '\0'
which we do not need to take account for here)?

> +
>  /* Generate the human-readable hex representation of an apr_uint64_t
>   * (basically a faster version of 'sprintf("%llx")')
>   */
> @@ -168,7 +170,7 @@ AP_DECLARE(char *) ap_make_etag_ex(reque
>  }
>  
>  etag = apr_palloc(r->pool, weak_len + sizeof("\"\"") +
> -4*(APR_SHA1_DIGESTSIZE/3) + vlv_len + 4);
> + SHA1_DIGEST_BASE64_LEN + vlv_len + 4);

Tab issue?
And why + 4? I see the need for

- a '\0' at the end
- I assume that vlv contains the closing '"', hence this is accounted for in 
vlv_len if a vlv is present
- sizeof("\"\"") takes care of the leading '"' (after a possible weak string) 
and of the closing '"' in case of no vlv
  and of the ';' in case of a vlv.

This would result in + 2.


Regards

Rüdiger

Re: svn commit: r1862270 - in /httpd/httpd/trunk/modules/dav: fs/dbm.c fs/repos.c main/mod_dav.c main/props.c main/std_liveprop.c main/util.c

[Ruediger Pluem]

On 6/29/20 5:52 PM, Graham Leggett wrote:
> On 28 Jun 2019, at 10:50, rpl...@apache.org  wrote:
> 
>> * Replace apr_psprintf with apr_pstrcat where the format strings only
>>  contain %s to improve efficiency. Leave out error messages as they
>>  are not on a crtical code path and error message become less readable
>>  when taking out the format specifiers.
> 
> I’ve proposed this for backport, as it blocks other dav changes. Would it be 
> possible to take a look?

Voted.

Regards

Rüdiger

Fixed: apache/httpd#898 (trunk - a1ffe6b)

[Travis CI]

Build Update for apache/httpd
-

Build: #898
Status: Fixed

Duration: 16 mins and 45 secs
Commit: a1ffe6b (trunk)
Author: Graham Leggett
Message: Make sure the get and restore the file offset when conputing the ETag. 
Be defensive
when opening the file.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1879332 
13f79535-47bb-0310-9956-ffa450edef68

View the changeset: 
https://github.com/apache/httpd/compare/134606e78062...a1ffe6bfede1

View the full build log and details: 
https://travis-ci.org/github/apache/httpd/builds/703163178?utm_medium=notification&utm_source=email


--

You can unsubscribe from build emails from the apache/httpd repository going to 
https://travis-ci.org/account/preferences/unsubscribe?repository=69847&utm_medium=notification&utm_source=email.
Or unsubscribe from *all* email updating your settings at 
https://travis-ci.org/account/preferences/unsubscribe?utm_medium=notification&utm_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.

Re: svn commit: r1862270 - in /httpd/httpd/trunk/modules/dav: fs/dbm.c fs/repos.c main/mod_dav.c main/props.c main/std_liveprop.c main/util.c

[Graham Leggett]

On 28 Jun 2019, at 10:50, rpl...@apache.org wrote:

> * Replace apr_psprintf with apr_pstrcat where the format strings only
>  contain %s to improve efficiency. Leave out error messages as they
>  are not on a crtical code path and error message become less readable
>  when taking out the format specifiers.

I’ve proposed this for backport, as it blocks other dav changes. Would it be 
possible to take a look?

Regards,
Graham
—

Re: svn commit: r1879285 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml include/ap_mmn.h include/http_core.h include/http_protocol.h include/httpd.h modules/dav/fs/repos.c modules/test/mod_

[Ruediger Pluem]

On 6/29/20 4:08 PM, Graham Leggett wrote:
> On 29 Jun 2020, at 11:41, Ruediger Pluem  > wrote:
> 
>>> +    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.
> 
> It would also mean it could no longer be optimised to a constant. What side 
> do we want to fall on?

Fair enough. The above is faster. Nevertheless I would prefer to group that 
better. Probably a separate define
SHA1_DIGEST_BASE64_LEN that only defines the base64 length of the base64 encode 
SHA1 digest. This would mean
more calc work for the compiler and not during runtime, but I guess this is 
acceptable.

> 
>>> +
>>> +    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?
> 
> It makes sense, but I would want to change something like this separately.

Makes sense.
Do you see a possibility to merge this code and the one of ap_md5digest to a 
more generic procedure that
allows to choose the digest algorithm while using 'MMAPED' reads?
BTW: Is sha1 mandatory for strong etags? If not wouldn't MD5 be enough and if 
MD5 is seen as too insecure
why isn't sha1?

>>> +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?
>
> I am guessing that the why is lost in the mists of time. As a guess, it 
> avoids an additional call.
>
> Using the actual file pointer is cleaner, as there is a guarantee that the 
> function does not have any side effects.
>
> http://svn.apache.org/viewvc?rev=1879332&view=rev

Thanks. Another option that just came to mind would it be also fine in case 
that we get an fd passed to do
an apr_file_dup() on that descriptor such that we also do not touch the actual 
buffering (if enabled) of the used fd?
Furthermore don't we need to do a seek to 0 before we start reading? Who tells 
us that the filepointer is at 0 for a passed fd?


Regards

Rüdiger

Re: svn commit: r1879285 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml include/ap_mmn.h include/http_core.h include/http_protocol.h include/httpd.h modules/dav/fs/repos.c modules/test/mod_

[Graham Leggett]

On 29 Jun 2020, at 14:49, Yann Ylavic  wrote:

>> Yes we can and should (but in separate commits).
>> 
>> I have my eye on the r->proxyreq flag, we can pack this into the binary 
>> notes too, values don’t need to be one bit wide.
> 
> Actually I was thinking the other way around, have the new "unsigned
> int strong_etag:1" bitfield rather than changing the existing ones...
> Why adding complexity with bit(s) macros while bitfields exist?

The problem with bitfields in the public APIs is that they’re not binary 
compatible across compilers. While it is very rare that a module will be built 
with a different compiler than httpd was, it’s still theoretically possible, 
and we should probably avoid it. Bitfields aren’t a problem for in-module or 
in-core code.

Regards,
Graham
—

Re: svn commit: r1879285 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml include/ap_mmn.h include/http_core.h include/http_protocol.h include/httpd.h modules/dav/fs/repos.c modules/test/mod_

[Graham Leggett]

On 29 Jun 2020, at 11:41, Ruediger Pluem  wrote:

>> +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.

It would also mean it could no longer be optimised to a constant. What side do 
we want to fall on?

>> +
>> +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?

It makes sense, but I would want to change something like this separately.

>> +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?

I am guessing that the why is lost in the mists of time. As a guess, it avoids 
an additional call.

Using the actual file pointer is cleaner, as there is a guarantee that the 
function does not have any side effects.

http://svn.apache.org/viewvc?rev=1879332&view=rev 


Regards,
Graham
—

Announcing ApacheCon @Home 2020

[Rich Bowen]

Hi, Apache enthusiast!

(You’re receiving this because you’re subscribed to one or more dev or 
user mailing lists for an Apache Software Foundation project.)


The ApacheCon Planners and the Apache Software Foundation are pleased to 
announce that ApacheCon @Home will be held online, September 29th 
through October 1st, 2020. We’ll be featuring content from dozens of our 
projects, as well as content about community, how Apache works, business 
models around Apache software, the legal aspects of open source, and 
many other topics.


Full details about the event, and registration, is available at 
https://apachecon.com/acah2020


Due to the confusion around how and where this event was going to be 
held, and in order to open up to presenters from around the world who 
may previously have been unable or unwilling to travel, we’ve reopened 
the Call For Presentations until July 13th. Submit your talks today at 
https://acna2020.jamhosted.net/


We hope to see you at the event!
Rich Bowen, VP Conferences, The Apache Software Foundation

Re: svn commit: r1879285 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml include/ap_mmn.h include/http_core.h include/http_protocol.h include/httpd.h modules/dav/fs/repos.c modules/test/mod_

[Yann Ylavic]

On Mon, Jun 29, 2020 at 1:59 PM Graham Leggett  wrote:
>
> On 29 Jun 2020, at 13:08, Yann Ylavic  wrote:
>
> >> /**
> >>  * @defgroup module_magic Module Magic mime types
> >> @@ -1097,6 +1138,11 @@ struct request_rec {
> >>  *  TODO: compact elsewhere
> >>  */
> >> unsigned int flushed:1;
> >> +/** Request flags associated with this request. Use
> >> + * AP_REQUEST_GET_FLAGS() and AP_REQUEST_SET_FLAGS() to access
> >> + * the elements of this field.
> >> + */
> >> +ap_request_bnotes_t bnotes;
> >> };
> >
> > Can't we use a single bitfield (like "flushed" above) for the single
> > AP_REQUEST_STRONG_ETAG flag?
>
> Yes we can and should (but in separate commits).
>
> I have my eye on the r->proxyreq flag, we can pack this into the binary notes 
> too, values don’t need to be one bit wide.

Actually I was thinking the other way around, have the new "unsigned
int strong_etag:1" bitfield rather than changing the existing ones...
Why adding complexity with bit(s) macros while bitfields exist?


Regards;
Yann.

Re: svn commit: r1879285 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml include/ap_mmn.h include/http_core.h include/http_protocol.h include/httpd.h modules/dav/fs/repos.c modules/test/mod_

[Graham Leggett]

On 29 Jun 2020, at 13:08, Yann Ylavic  wrote:

>> /**
>>  * @defgroup module_magic Module Magic mime types
>> @@ -1097,6 +1138,11 @@ struct request_rec {
>>  *  TODO: compact elsewhere
>>  */
>> unsigned int flushed:1;
>> +/** Request flags associated with this request. Use
>> + * AP_REQUEST_GET_FLAGS() and AP_REQUEST_SET_FLAGS() to access
>> + * the elements of this field.
>> + */
>> +ap_request_bnotes_t bnotes;
>> };
> 
> Can't we use a single bitfield (like "flushed" above) for the single
> AP_REQUEST_STRONG_ETAG flag?

Yes we can and should (but in separate commits).

I have my eye on the r->proxyreq flag, we can pack this into the binary notes 
too, values don’t need to be one bit wide.

Regards,
Graham
—

Re: svn commit: r1879285 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml include/ap_mmn.h include/http_core.h include/http_protocol.h include/httpd.h modules/dav/fs/repos.c modules/test/mod_

[Yann Ylavic]

On Sun, Jun 28, 2020 at 1:41 AM  wrote:
>
> Author: minfrin
> Date: Sat Jun 27 23:41:00 2020
> New Revision: 1879285
>
> URL: http://svn.apache.org/viewvc?rev=1879285&view=rev
[]
> --- httpd/httpd/trunk/include/httpd.h (original)
> +++ httpd/httpd/trunk/include/httpd.h Sat Jun 27 23:41:00 2020
[]
> +/**
> + * @defgroup bnotes Binary notes recognized by the server
> + * @ingroup APACHE_CORE_DAEMON
> + * @{
> + *
> + * @brief Binary notes recognized by the server.
> + */
> +
> +/**
> + * The type used for request binary notes.
> + */
> +typedef apr_uint64_t ap_request_bnotes_t;
> +
> +/**
> + * These constants represent bitmasks for notes associated with this
> + * request. There are space for 64 bits in the apr_uint64_t.
> + *
> + */
> +#define AP_REQUEST_STRONG_ETAG 1 >> 0
> +
> +/**
> + * This is a convenience macro to ease with getting specific request
> + * binary notes.
> + */
> +#define AP_REQUEST_GET_BNOTE(r, mask) \
> +((mask) & ((r)->bnotes))
> +
> +/**
> + * This is a convenience macro to ease with setting specific request
> + * binary notes.
> + */
> +#define AP_REQUEST_SET_BNOTE(r, mask, val) \
> +(r)->bnotes = (((r)->bnotes & ~(mask)) | (val))
> +
> +/**
> + * Returns true if the strong etag flag is set for this request.
> + */
> +#define AP_REQUEST_IS_STRONG_ETAG(r) \
> +AP_REQUEST_GET_BNOTE((r), AP_REQUEST_STRONG_ETAG)
> +/** @} */
> +
>
>  /**
>   * @defgroup module_magic Module Magic mime types
> @@ -1097,6 +1138,11 @@ struct request_rec {
>   *  TODO: compact elsewhere
>   */
>  unsigned int flushed:1;
> +/** Request flags associated with this request. Use
> + * AP_REQUEST_GET_FLAGS() and AP_REQUEST_SET_FLAGS() to access
> + * the elements of this field.
> + */
> +ap_request_bnotes_t bnotes;
>  };

Can't we use a single bitfield (like "flushed" above) for the single
AP_REQUEST_STRONG_ETAG flag?


Regards;
Yann.

Re: mod_proxy_fcgi bug using CONTENT_LENGTH and Transfer-Encoding chunked

[Oliver Dunk]

Luca, thanks for the response. What you’re saying makes total sense, so no 
worries at all.

Tom, I won’t try to answer your questions, because I could only provide an 
educated guess at the answers :)

> On 27 Jun 2020, at 14:18, Tom Browder  wrote:
> 
> On Sat, Jun 27, 2020 at 07:54 Luca Toscano  > wrote:
> Hi Oliver,
> 
> your request was duly noted, we might get somebody to look at this
> during the next days/weeks, but we can't promise anything. The bug is
> sadly not as simple as adding a line of code (see comments in bz
> 57087), plus the current dev resources have limited bandwidth.
> 
> I was just investigating moving to fast cgi from cgi and this caught my 
> attention.
> 
> Will affect non-php code? Can I avoid chunking files if necessary to work 
> around the bug?
> 
> Thanks.
> 
> Best regards,
> 
> -Tom
> 

Re: Still Failing: apache/httpd#896 (trunk - 9af2218)

[Joe Orton]

On Mon, Jun 29, 2020 at 11:16:54AM +0200, Graham Leggett wrote:
> On 29 Jun 2020, at 09:19, Joe Orton  wrote:
> 
> > The litmus tests are failing, not the perl-framework tests:
> > 
> > https://travis-ci.org/github/apache/httpd/jobs/702768269#L2491
> 
> Ah - that’s where  I was going wrong.
> 
> > You can also see that there are segfaults logged from line 2519 onwards:
> > 
> > https://travis-ci.org/github/apache/httpd/jobs/702768269#L2519
> > 
> > Running litmus locally, the backtrace looks like this:
> 
> So the simple workaround is to be defensive against ctx->r being NULL, 
> but I need to check - is there ever a legitimate reason for there to 
> be a filled out ctx structure including a filename, but with no 
> request?

I don't know, you added the ctx->r field in r823703 and it's not obvious 
exactly what the semantics are supposed to be.

If the request_rec is supposed to correspond to the dav_resource object 
then it makes sense that it is NULL when looking at the dav_resource 
representing the parent collection (in this case) or some other resource 
during a walk.  It would be good to document this!

Regards, Joe

Re: svn commit: r1879307 - /httpd/httpd/trunk/modules/dav/main/mod_dav.c

[Graham Leggett]

On 29 Jun 2020, at 12:10, Ruediger Pluem  wrote:

>> +result = dav_run_deliver_report(r, resource, doc,
>> +r->output_filters, &err);
>> +switch (result) {
>> +case OK:
>> return DONE;
> 
> What happens if err != NULL. Should we handle that here like in the default 
> case or
> should the hook implementer ensure to not return OK if err != NULL?

Hmmm… this needs tightening up.

>> +case DECLINED:
>> +/* No one handled the report */
>> +return HTTP_NOT_IMPLEMENTED;
> 
> Previously we were returning DECLINED in case there was no vsn_hooks and thus 
> nothing to report. Now we return
> HTTP_NOT_IMPLEMENTED. Why this change?

When mod_dav was written, it was assumed that the REPORT method was exclusive 
to https://tools.ietf.org/html/rfc3253. As a result, if a versioning 
implementation was present, the versioning implementation (eg svn) was expected 
to handle REPORT, and no other.

Other WebDAV extensions arrived, and they also used REPORT. But if a versioning 
implementation was present, that implementation would consume the report, not 
recognise the report, and then return an error. The mod_caldav code that Nokia 
worked on in the late 2000s hacked around this by reading the REPORT body 
first, then creating an input filter to re-insert the body if mod_caldav 
realised the REPORT wasn’t for it. Parsing XML bodies over and over is very 
ugly.

What this change does is formally recognise in code that multiple RFCs handle 
the REPORT method. We parse the XML body, then offer this parsed body to anyone 
who wants it via the deliver_report hook. Modules get their chance to handle 
the report. As a last resort, the mod_dav core passes the body to the 
versioning implementaton (eg svn) which then responds as previous, maintaining 
existing behaviour.

To answer you direct question above, at the point we return 
HTTP_NOT_IMPLEMENTED we have consumed the REPORT body. We can’t return DECLINED 
in this case.

There are a significant number of RFCs that mod_dav doesn't support, such as 
https://tools.ietf.org/html/rfc5689. We have RFC compliance work to do.

Regards,
Graham
—

Re: svn commit: r1879307 - /httpd/httpd/trunk/modules/dav/main/mod_dav.c

[Ruediger Pluem]

On 6/28/20 3:28 PM, minf...@apache.org wrote:
> Author: minfrin
> Date: Sun Jun 28 13:28:19 2020
> New Revision: 1879307
> 
> URL: http://svn.apache.org/viewvc?rev=1879307&view=rev
> Log:
> Add implementation of deliver_report and gather_reports to mod_dav.c.
> 
> Modified:
> httpd/httpd/trunk/modules/dav/main/mod_dav.c
> 
> Modified: httpd/httpd/trunk/modules/dav/main/mod_dav.c
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/mod_dav.c?rev=1879307&r1=1879306&r2=1879307&view=diff
> ==
> --- httpd/httpd/trunk/modules/dav/main/mod_dav.c (original)
> +++ httpd/httpd/trunk/modules/dav/main/mod_dav.c Sun Jun 28 13:28:19 2020

> @@ -4228,24 +4271,36 @@ static int dav_method_report(request_rec
>  ap_set_content_type(r, DAV_XML_CONTENT_TYPE);
>  
>  /* run report hook */
> -if ((err = (*vsn_hooks->deliver_report)(r, resource, doc,
> -r->output_filters)) != NULL) {
> -if (! r->sent_bodyct)
> -  /* No data has been sent to client yet;  throw normal error. */
> -  return dav_handle_err(r, err, NULL);
> -
> -/* If an error occurred during the report delivery, there's
> -   basically nothing we can do but abort the connection and
> -   log an error.  This is one of the limitations of HTTP; it
> -   needs to "know" the entire status of the response before
> -   generating it, which is just impossible in these streamy
> -   response situations. */
> -err = dav_push_error(r->pool, err->status, 0,
> - "Provider encountered an error while streaming"
> - " a REPORT response.", err);
> -dav_log_err(r, err, APLOG_ERR);
> -r->connection->aborted = 1;
> +result = dav_run_deliver_report(r, resource, doc,
> +r->output_filters, &err);
> +switch (result) {
> +case OK:
>  return DONE;

What happens if err != NULL. Should we handle that here like in the default 
case or
should the hook implementer ensure to not return OK if err != NULL?

> +case DECLINED:
> +/* No one handled the report */
> +return HTTP_NOT_IMPLEMENTED;

Previously we were returning DECLINED in case there was no vsn_hooks and thus 
nothing to report. Now we return
HTTP_NOT_IMPLEMENTED. Why this change?

> +default:
> +if ((err) != NULL) {
> +
> +if (! r->sent_bodyct) {
> +  /* No data has been sent to client yet;  throw normal error. */
> +  return dav_handle_err(r, err, NULL);
> +}
> +
> +/* If an error occurred during the report delivery, there's
> +   basically nothing we can do but abort the connection and
> +   log an error.  This is one of the limitations of HTTP; it
> +   needs to "know" the entire status of the response before
> +   generating it, which is just impossible in these streamy
> +   response situations. */
> +err = dav_push_error(r->pool, err->status, 0,
> + "Provider encountered an error while 
> streaming"
> + " a REPORT response.", err);
> +dav_log_err(r, err, APLOG_ERR);
> +r->connection->aborted = 1;
> +
> +return DONE;
> +}
>  }
>  
>  return DONE;

Regards

Rüdiger

Re: svn commit: r1879285 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/core.xml include/ap_mmn.h include/http_core.h include/http_protocol.h include/httpd.h modules/dav/fs/repos.c modules/test/mod_

[Ruediger Pluem]

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, &c

Re: Still Failing: apache/httpd#896 (trunk - 9af2218)

[Graham Leggett]

On 29 Jun 2020, at 09:19, Joe Orton  wrote:

> The litmus tests are failing, not the perl-framework tests:
> 
> https://travis-ci.org/github/apache/httpd/jobs/702768269#L2491

Ah - that’s where  I was going wrong.

> You can also see that there are segfaults logged from line 2519 onwards:
> 
> https://travis-ci.org/github/apache/httpd/jobs/702768269#L2519
> 
> Running litmus locally, the backtrace looks like this:

So the simple workaround is to be defensive against ctx->r being NULL, but I 
need to check - is there ever a legitimate reason for there to be a filled out 
ctx structure including a filename, but with no request?

Process 9883 stopped
* thread #62, stop reason = EXC_BAD_ACCESS (code=1, address=0x58)
frame #0: 0x000100203e09 
httpd`dav_fs_getetag(resource=0x00010106f1e0) at repos.c:1869:31
   1866 }
   1867 
   1868 er.vlist_validator = NULL;
-> 1869 er.request_time = ctx->r->request_time;
   1870 er.finfo = &ctx->finfo;
   1871 er.pathname = ctx->pathname;
   1872 er.fd = NULL;
Target 0: (httpd) stopped.
(lldb) print ctx
(dav_resource_private *) $0 = 0x00010106f110
(lldb) print *ctx
(dav_resource_private) $1 = {
  pool = 0x000105098628
  pathname = 0x00010106f198 
"/Users/minfrin/src/apache/sandbox/proxy/htdocs/dav/litmus/lockcoll"
  finfo = {
pool = 0x000105098628
valid = 7598960
protection = 1877
filetype = APR_DIR
user = 501
group = 20
inode = 61244902
device = 16777220
nlink = 2
size = 64
csize = 0
atime = 159342121800
mtime = 159342121800
ctime = 159342121800
fname = 0x00010106f198 
"/Users/minfrin/src/apache/sandbox/proxy/htdocs/dav/litmus/lockcoll"
name = 0x
filehand = 0x
  }
  r = 0x
}
(lldb) 

Regards,
Graham
—

Re: iOS 14 / macOS 11 and HTTP/3 support

[Stefan Eissing]

> Am 28.06.2020 um 18:03 schrieb William A Rowe Jr :
> 
> On Sun, Jun 28, 2020 at 5:53 AM Graham Leggett  wrote:
> On 27 Jun 2020, at 14:48, Luca Toscano  wrote:
> 
> > the challenges are the same one discussed in your previous email
> > thread 
> > (https://lists.apache.org/thread.html/eb086eafbd9309eb1efedac3bf3dcc410a95d06206c97e7ade01c254%40%3Cdev.httpd.apache.org%3E).
> > I think that everybody would love to start working/helping on adding
> > HTTP/3 support but the work to be done is huge, involves invasive
> > changes to the httpd's source code and the current dev resources don't
> > have (rightfully) bandwidth to support the current codebase and plan a
> > major refactoring.
> 
> I would be careful with wide reaching statements like this.
> 
> I’ve been working on identifying and removing blockers in various parts of 
> the httpd subsystems that prevent httpd to be cleanly event driven, and most 
> of those blockers have been removed.
> 
> The underlying architecture of httpd is very strong, and would support new 
> protocols without too much trouble.
> 
> The main point is that it must be done carefully and properly, but this is 
> not a reason to not do it at all.

Apache httpd is a very strong HTTP/1.x server. And your efforts to make 
response streaming more event driven have been great. 

However it has resulted in code that is, in my impression on talks on the dev 
list regarding Yann's changes, very hard to read and improve. Not impossible, 
but not for the meek. And not because it is written badly, but because the 
state handling and interactions between buckets and pools is complex and full 
of pitfalls.
But if it's done properly, as you say, it works nicely.

For anyone thinking about bringing h3 into the server, consider your options:
1. Think of a mod_h3 as its own h3->h1 proxy. Open a h1 connection for every 
quic stream, write a h1 request, read (often event based, often not) a h1 
response from it, transform this to your h3 answer. Stay away from touching the 
server's h1 stasis field - cope with it.
2. Redesign a FOSS httpd v3.x with a new architecture, embracing a non-blocking 
processing model. Maybe under a different name.
3. Wait for someone else to do 1 or 2.

Cheers, Stefan

Re: Still Failing: apache/httpd#896 (trunk - 9af2218)

[Joe Orton]

On Sun, Jun 28, 2020 at 05:15:03PM +0200, Graham Leggett wrote:
> On 28 Jun 2020, at 15:47, Travis CI  wrote:
> 
> > apache / httpd
> > trunk
> > Build #896 is still failing9 mins and 44 secs
> 
> Travis mavens, I’ve been looking for the test/perl-framework directory 
> as referred to in --with-test-suite=test/perl-framework but I’m coming 
> up with a blank.

The litmus tests are failing, not the perl-framework tests:

https://travis-ci.org/github/apache/httpd/jobs/702768269#L2491

You can also see that there are segfaults logged from line 2519 onwards:

https://travis-ci.org/github/apache/httpd/jobs/702768269#L2519

Running litmus locally, the backtrace looks like this:

warning: Unexpected size of section `.reg-xstate/44301' in core file.
#0  0x7fcea5f9059a in dav_fs_getetag (resource=) at 
repos.c:1869
1869er.request_time = ctx->r->request_time;
[Current thread is 1 (Thread 0x7fcea6d70800 (LWP 44301))]
Missing separate debuginfos, use: dnf debuginfo-install 
pcre2-10.35-3.fc32.x86_64
(gdb) where
#0  0x7fcea5f9059a in dav_fs_getetag (resource=) at 
repos.c:1869
#1  0x7fcea5fda7af in dav_validate_resource_state (p=0xfb5288, 
resource=, lockdb=0xfcf060, if_header=0xfcf008, 
flags=flags@entry=288, pbuf=pbuf@entry=0x7ffdc5916010, r=0xfb5300) at 
util.c:1046
#2  0x7fcea5fdb9cf in dav_validate_request (r=r@entry=0xfb5300, 
resource=0xfcec70, depth=depth@entry=0, 
locktoken=locktoken@entry=0x0, response=response@entry=0x7ffdc5916168, 
flags=flags@entry=32, lockdb=)
at util.c:1652
#3  0x7fcea5fd1e4e in dav_method_put (r=0xfb5300) at mod_dav.c:985
#4  0x004170d0 in ap_run_handler (r=r@entry=0xfb5300) at config.c:170
#5  0x00417666 in ap_invoke_handler (r=r@entry=0xfb5300) at config.c:444
#6  0x0045a3eb in ap_process_async_request (r=r@entry=0xfb5300) at 
http_request.c:463
#7  0x0045a41f in ap_process_request (r=r@entry=0xfb5300) at 
http_request.c:498
#8  0x00456fbe in ap_process_http_sync_connection (c=0xf90440) at 
http_core.c:214
#9  ap_process_http_connection (c=0xf90440) at http_core.c:255
#10 0x004283c0 in ap_run_process_connection (c=c@entry=0xf90440) at 
connection.c:42
#11 0x00428919 in ap_process_connection (c=c@entry=0xf90440, 
csd=) at connection.c:219
#12 0x7fcea75439fe in child_main (child_num_arg=child_num_arg@entry=2, 
child_bucket=child_bucket@entry=0) at prefork.c:621
#13 0x7fcea7543d7e in make_child (s=, slot=2) at 
prefork.c:723
#14 0x7fcea75444bf in perform_idle_server_maintenance (p=) 
at prefork.c:827
#15 prefork_run (_pconf=, plog=, s=) at prefork.c:1020
#16 0x0042b7f0 in ap_run_mpm (pconf=pconf@entry=0x9f33f8, 
plog=0xa1e608, s=0xa1a750) at mpm_common.c:101
#17 0x00415347 in main (argc=, argv=) at 
main.c:844

Re: svn commit: r1879253 - /httpd/httpd/trunk/server/util_script.c

[Ruediger Pluem]

On 6/27/20 11:11 AM, elu...@apache.org wrote:
> Author: elukey
> Date: Sat Jun 27 09:11:32 2020
> New Revision: 1879253
> 
> URL: http://svn.apache.org/viewvc?rev=1879253&view=rev
> Log:
> server/util_script.c: tune logging Last-Modified header
> 
> Follow up after Joe's feedback in STATUS:
>   - If APR_DATE_BAD is returned for Last-Modified, log it at INFO level
> (as opposed to trace).
>   - Remove unnecessary guard for APLOGrtrace1(r).


As this is now a APLOG_INFO, can you please add an APLOGNO?
For further details please have a look at docs/log-message-tags/README

Regards

Rüdiger