Re: AP_CONN_CLOSE on force-response-1.0

[Aleksey Midenkov]

On Wednesday 10 October 2007 23:44:03 Roy T. Fielding wrote:
> On Oct 10, 2007, at 7:08 AM, Aleksey Midenkov wrote:
> > On Wednesday 10 October 2007 18:04:47 Jim Jagielski wrote:
> >> On Oct 10, 2007, at 9:38 AM, Aleksey Midenkov wrote:
> >>>  And resolution for those who will suffer can be
> >>>
> >>> SetEnvIf Request_Protocol HTTP/1.0 nokeepalive
> >>>
> >>> No unnecessary CPU processing for majority.
> >>
> >> Huh? You're adding another conditional that needs
> >> to be checked... And for most cases, where the protocol
> >> is 1.1, it would be a waste of time.
> >
> > For one installation from a million it is acceptable tradeoff. But
> > I really
> > doubt it will be necessary to anyone.
>
> This is all irrelevant.  No current installation should need any of
> those env variables set.  They exist solely for working around old
> versions of old clients that no longer exist on the net.
>
> Roy

Not all... We need mod_proxy responding exactly with what protocol backend 
server sent. For this purpose we use force-response-1.0.

Re: Broken URI-unescaping in mod_proxy

[rahul]

[Graham Leggett:]
| > It would be nice to have different modules for reverse proxy and forward
| > proxy.. from an FTP perspective.
| >
| > There is a fairly large difference in FTP (and perhaps in other protocols
| > too) in terms of the optimizations that needs to be done for forward proxy
| > and reverse proxy.
| >
| > In forward proxy, we can not assume the kind of ftp servers the client
| > requests. So when there is an error of some sort we should try again
| > with a syntax that might be acceptable to the next possible type of
| > server.
| >
| > In the reverse proxy, this is wrong, and introduces unnecessary
| > overheads in network traffic (where it would be simpler to ask the user
| > to provide the type of server in the backend and error out if the ftp
| > server returns error.)
| 
| There is no need to have separate module to achieve this - providing a
| mechanism to override certain behaviour when the administrator wants it,
| but defaulting to RFC compliant behaviour will achieve the same thing.

True, my point is that these choices are distributed all over the code.
While it can certainly be run together, it would be much cleaner to have
different modules with emphasis on different things while using a common
ftp_util base for things that are similar.

Another problem is with the default behavior. What is nice for a forward
ftp proxy is not a correct default for a reverse ftp proxy (as explained
above).

Thirdly, the FTP rfc is silent (AFAIK - please correct me if I am wrong.)
in things like LIST format. so there is no RFC compliant behavior to
default to for this. 

rahul
--
1. e4 _

Re: Broken URI-unescaping in mod_proxy

[rahul]

[Graham Leggett:]
| > It would be nice to have different modules for reverse proxy and forward
| > proxy.. from an FTP perspective.
| >
| > There is a fairly large difference in FTP (and perhaps in other protocols
| > too) in terms of the optimizations that needs to be done for forward proxy
| > and reverse proxy.
| >
| > In forward proxy, we can not assume the kind of ftp servers the client
| > requests. So when there is an error of some sort we should try again
| > with a syntax that might be acceptable to the next possible type of
| > server.
| >
| > In the reverse proxy, this is wrong, and introduces unnecessary
| > overheads in network traffic (where it would be simpler to ask the user
| > to provide the type of server in the backend and error out if the ftp
| > server returns error.)
|
| There is no need to have separate module to achieve this - providing a
| mechanism to override certain behaviour when the administrator wants it,
| but defaulting to RFC compliant behaviour will achieve the same thing.

True, my point is that these choices are distributed all over the code.
While it can certainly be run together, it would be much cleaner to have
different modules with emphasis on different things while using a common
ftp_util base for things that are similar.

Another problem is with the default behavior. What is nice for a forward
ftp proxy is not a correct default for a reverse ftp proxy (as explained
above).

Thirdly, the FTP rfc is silent (AFAIK - please correct me if I am wrong.)
in things like LIST format. so there is no RFC compliant behavior to
default to for this.

rahul
--
1. e4 _

Re: Broken URI-unescaping in mod_proxy

[Graham Leggett]

On Thu, October 11, 2007 12:53 pm, rahul wrote:

> True, my point is that these choices are distributed all over the code.
> While it can certainly be run together, it would be much cleaner to have
> different modules with emphasis on different things while using a common
> ftp_util base for things that are similar.

The ftp stuff should only be contained in mod_proxy_ftp, which is itself a
submodule of mod_proxy, these changes should definitely not be all over
the code.

If you split mod_proxy_ftp into two, you now have two almost identical
modules where one half contains a feature and the other half doesn't,
leading to situations where something is supported in the reverse proxy
case, but not the forward proxy case. I don't see any value in this.

> Another problem is with the default behavior. What is nice for a forward
> ftp proxy is not a correct default for a reverse ftp proxy (as explained
> above).
>
> Thirdly, the FTP rfc is silent (AFAIK - please correct me if I am wrong.)
> in things like LIST format. so there is no RFC compliant behavior to
> default to for this.

The ftp proxy handles this by following "safe" behaviour, such as "cd"ing
individually to each directory component instead of assuming a potentially
incorrect path separator. This makes the proxy work everywhere, but in
cases where an admin knows what kind of proxy is being used, it's quite
sensible to offer the admin the option to hard code a separator, or hard
code a list format, so as to cut down on network traffic. None of this
justifies a split in modules though.

Please don't forget that mod_proxy already contains a non trivial
structure: mod_proxy is a "parent" module, providing hooks to
mod_proxy_http and mod_proxy_ftp, which are child modules. Splitting the
modules yet again adds a documentation and support burden - now people
must be educated on why they must choose module A instead of almost
identical module B.

Regards,
Graham
--

RE: Error from DSOLoadLibrary in Apache2.0.61(64 bit) on AIX 5.3(64 bit)

[Renu Tiwari]

While building apache source on AIX5.3 when I give ./configure command 
following message is shown.


Checking for DSO...
checking for NSLinkModule... no
checking for shl_load in -ldld... no
checking for dlopen... yes
checking for dlsym... yes


Will "NSLinkModule" and "shl_load in -ldld"  create any issues in dynamic 
loading of shared object?

Thanks


From: Jeff Trawick [mailto:[EMAIL PROTECTED]
Sent: Tuesday, October 09, 2007 7:16 PM
To: dev@httpd.apache.org
Subject: Re: Error from DSOLoadLibrary in Apache2.0.61(64 bit) on AIX 5.3(64 
bit)

On 10/9/07, Renu Tiwari <[EMAIL PROTECTED]> wrote:
Hi

Thanks for the reply.

Just wanted to confirm one thing.
We have build the apache 2.0.61 source on AIX 5.2(64 bit) and than placed the 
resultant Apache folder structure to AIX5.3(64 bit) m/c.

Can this cause some kind of discrepencies?

It shouldn't ;)


 CAUTION - Disclaimer *
This e-mail contains PRIVILEGED AND CONFIDENTIAL INFORMATION intended solely 
for the use of the addressee(s). If you are not the intended recipient, please 
notify the sender by e-mail and delete the original message. Further, you are 
not to copy, disclose, or distribute this e-mail or its contents to any other 
person and any such actions are unlawful. This e-mail may contain viruses. 
Infosys has taken every reasonable precaution to minimize this risk, but is not 
liable for any damage you may sustain as a result of any virus in this e-mail. 
You should carry out your own virus checks before opening the e-mail or 
attachment. Infosys reserves the right to monitor and review the content of all 
messages sent to or from this e-mail address. Messages sent to or from this 
e-mail address may be stored on the Infosys e-mail system.
***INFOSYS End of Disclaimer INFOSYS***

Re: Broken URI-unescaping in mod_proxy

[rahul]

[Graham Leggett:]
| > True, my point is that these choices are distributed all over the code.
| > While it can certainly be run together, it would be much cleaner to have
| > different modules with emphasis on different things while using a common
| > ftp_util base for things that are similar.
 
| The ftp stuff should only be contained in mod_proxy_ftp, which is itself a
| submodule of mod_proxy, these changes should definitely not be all over
| the code.

Since the discussion was about merits of splitting mod_proxy into reverse
and forward proxy modules, I assumed that the corresponding for ftp too.

| If you split mod_proxy_ftp into two, you now have two almost identical
| modules where one half contains a feature and the other half doesn't,
| leading to situations where something is supported in the reverse proxy
| case, but not the forward proxy case. I don't see any value in this.

Things that are identical can be factored out (as I mentioned) into a
separate util file.

| > Another problem is with the default behavior. What is nice for a forward
| > ftp proxy is not a correct default for a reverse ftp proxy (as explained
| > above).
| >
| > Thirdly, the FTP rfc is silent (AFAIK - please correct me if I am wrong.)
| > in things like LIST format. so there is no RFC compliant behavior to
| > default to for this.
| 
| The ftp proxy handles this by following "safe" behaviour, such as "cd"ing
| individually to each directory component instead of assuming a potentially
| incorrect path separator. This makes the proxy work everywhere, but in
| cases where an admin knows what kind of proxy is being used, it's quite
| sensible to offer the admin the option to hard code a separator, or hard
| code a list format, so as to cut down on network traffic. None of this
| justifies a split in modules though.


This is one place. (LIST/NLST) where different systems vary widely in
their implementation.

Another is how you interpret the result of SIZE command (not in RFC,
but most implement it - so if it works it is a file if it does not it
_may_ be a directory.)

A different one is what kind of connection you use to transfer data,
ie PASSIVE/EPASV/PORT,..

One more is how you do CWD as you have mentioned above.

Another is that you may want to restrict the ftp server you front end to
some users with out going to the ftp server to verify.


In the case of a forward proxy you cant expect to get any of these
information from the configuration. So you start with the most general
assumption, and if it fails, you try the next one by interrogating the
server. The requirement of highest importance is to try to find a
sequence of commands that matches the request and serve it.

For a reverse proxy, the scenario changes. You assume with a default
assumption which may (most possibly) partially be supplied by the
configuration, Thus if the ftp server returns an error on any thing, you
error out and return with out trying any thing else. The requirement
here is to avoid loading the FTP server you are front ending too much.

The conflict between these two interests can be seen in all the places I
mentioned.


I think a better way to do this is to code it around two state-machines,
one each for RFP and FFP. The STM should be responsible for what step to
take after each response/error from the server, If we do that then it will
be possible to localize the above decisions to just those with the rest
of FTP code being shared.

| Please don't forget that mod_proxy already contains a non trivial
| structure: mod_proxy is a "parent" module, providing hooks to
| mod_proxy_http and mod_proxy_ftp, which are child modules. Splitting the
| modules yet again adds a documentation and support burden - now people
| must be educated on why they must choose module A instead of almost
| identical module B.

IMHO the people generally think of their setup as either of a proxy or
as a load balancer/reverse proxy.  It would be easy for them to make the
connection. We do have a precedence in different directives having a large
overlap as in ProxyPass and Rewrite rules.

(OT):
In case you do have spare time, and working on the mod_proxy_ftp, could
you please review the patches (41676, 43231, 43275, 31490, 27835)? I had
submitted these some time back but does not seem to have garnered any
interest.

rahul
--
1. e4 _

[PATCH] Re: AP_CONN_CLOSE on force-response-1.0

[Aleksey Midenkov]

On Wednesday 10 October 2007 18:46:15 Jim Jagielski wrote:
> Or how about leaving the vast majority of the public completely
> unaffected and creating a new envvar for those who have problems
> with the 10 year old implementation...
>
> If, however, you come up with a complete patch, including docs,
> that do what you'd like, I'd certainly (assuming it's a valid
> patch) +1 it.

I'll prefer to remain of the same opinion. Thank You! Here is the patch. It is 
really simple to be invalid... :)
Index: docs/manual/env.html.en
===
--- docs/manual/env.html.en	(revision 583757)
+++ docs/manual/env.html.en	(working copy)
@@ -252,6 +252,12 @@
   implemented as a result of a problem with AOL's proxies. Some
   HTTP/1.0 clients may not behave correctly when given an HTTP/1.1
   response, and this can be used to interoperate with them.
+  
+  
+  For some time the behaviour of this variable was to disable
+  KeepAlive for HTTP/1.0 responses.
+  Now it does not. To achieve such an effect SetEnvIf can be used on nokeepalive variable.
+  
 
 
 
Index: docs/manual/env.xml
===
--- docs/manual/env.xml	(revision 583757)
+++ docs/manual/env.xml	(working copy)
@@ -292,6 +292,14 @@
   implemented as a result of a problem with AOL's proxies. Some
   HTTP/1.0 clients may not behave correctly when given an HTTP/1.1
   response, and this can be used to interoperate with them.
+  
+  
+  For some time the behaviour of this variable was to disable
+  KeepAlive for HTTP/1.0 responses.
+  Now it does not. To achieve such an effect SetEnvIf can be used on nokeepalive variable.
+  
 
 
 
Index: CHANGES
===
--- CHANGES	(revision 583757)
+++ CHANGES	(working copy)
@@ -264,6 +264,9 @@
  access control, as in: SSLRequire "value" in OID("1.3.6.1.4.1.18060.1")
  [Martin Kraemer, David Reid]
 
+  *) HTTP protocol: removed disabling Keep-Alive for HTTP/1.0 when
+ force-response-1.0 is in effect. [Aleksey Midenkov ]
+
   [Apache 2.1.0-dev includes those bug fixes and changes with the
Apache 2.2.xx tree as documented, and except as noted, below.]
 
Index: modules/http/http_filters.c
===
--- modules/http/http_filters.c	(revision 583757)
+++ modules/http/http_filters.c	(working copy)
@@ -673,7 +673,7 @@
 
 /*
  * Determine the protocol to use for the response. Potentially downgrade
- * to HTTP/1.0 in some situations and/or turn off keepalives.
+ * to HTTP/1.0 in some situatuations.
  *
  * also prepare r->status_line.
  */
@@ -702,7 +702,6 @@
 if (r->proto_num == HTTP_VERSION(1,0)
 && apr_table_get(r->subprocess_env, "force-response-1.0")) {
 *protocol = "HTTP/1.0";
-r->connection->keepalive = AP_CONN_CLOSE;
 }
 else {
 *protocol = AP_SERVER_PROTOCOL;

Re: Broken URI-unescaping in mod_proxy

[Graham Leggett]

On Thu, October 11, 2007 2:41 pm, rahul wrote:

> In the case of a forward proxy you cant expect to get any of these
> information from the configuration. So you start with the most general
> assumption, and if it fails, you try the next one by interrogating the
> server. The requirement of highest importance is to try to find a
> sequence of commands that matches the request and serve it.

As it does now, yes.

> For a reverse proxy, the scenario changes. You assume with a default
> assumption which may (most possibly) partially be supplied by the
> configuration, Thus if the ftp server returns an error on any thing, you
> error out and return with out trying any thing else. The requirement
> here is to avoid loading the FTP server you are front ending too much.
>
> The conflict between these two interests can be seen in all the places I
> mentioned.

As I said earlier, it is perfectly reasonable to add functionality that
allows the admin to specify concrete behaviour, so instead of "try A, and
if that fails try B" the admin can just say "try B, don't waste your time
trying A". This can be accomplished by a few lines of code in the existing
module.

> (OT):
> In case you do have spare time, and working on the mod_proxy_ftp, could
> you please review the patches (41676, 43231, 43275, 31490, 27835)? I had
> submitted these some time back but does not seem to have garnered any
> interest.

Let me make some time and take a look.

Regards,
Graham
--

Re: AP_CONN_CLOSE on force-response-1.0

[Roy T. Fielding]

On Oct 11, 2007, at 12:55 AM, Aleksey Midenkov wrote:

This is all irrelevant.  No current installation should need any of
those env variables set.  They exist solely for working around old
versions of old clients that no longer exist on the net.


Not all... We need mod_proxy responding exactly with what protocol  
backend

server sent. For this purpose we use force-response-1.0.


Then you are violating the HTTP standard.

Roy

Re: svn commit: r583829 - /httpd/httpd/branches/2.2.x/STATUS

[Jim Jagielski]

On Oct 11, 2007, at 3:12 PM, Ruediger Pluem wrote:




On 10/11/2007 04:12 PM, [EMAIL PROTECTED] wrote:

Author: jim
Date: Thu Oct 11 07:12:02 2007
New Revision: 583829

URL: http://svn.apache.org/viewvc?rev=583829&view=rev
Log:
Add fix for, as of now, unconfirmed issue...

Modified:
httpd/httpd/branches/2.2.x/STATUS

Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/ 
STATUS?rev=583829&r1=583828&r2=583829&view=diff
= 
=

--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Thu Oct 11 07:12:02 2007
@@ -207,6 +207,12 @@
  http://svn.apache.org/viewvc?view=rev&revision=582631
  +1: niq

+* modules/proxy/mod_proxy_http.c  
(ap_proxy_http_register_hook): Fix
+  apr_pool_cleanup_register() invocation added in r583202,  
which was

+  causing every apr_proc_create() call to segfault.
+  Trunk version of patch:
+http://svn.apache.org/viewvc?view=rev&revision=583813
+


I assume you are +1 on your proposal, right :-)?



The weird thing is that I didn't see the problem during
earlier testing (otherwise the +1 for r583202 wouldn't
have happened), so I'm doing deeper testing on recreating
the issue, but in anticipation, I added the proposal :)

Re: mod_proxy and interim responses

[Joe Orton]

On Thu, Oct 04, 2007 at 11:52:03AM +0100, Nick Kew wrote:
> On Thu, 04 Oct 2007 11:27:30 +0200
> Ruediger Pluem <[EMAIL PROTECTED]> wrote:
> 
> > I think you should move it to http_filters.c. There are a bunch
> > of static functions that you can use for creating the header strings
> > and all this stuff without reinventing the wheel.
> 
> Hmm.  It's only a one-line reinvention of a wheel.
> I guess you mean things like ... AP_SERVER_PROTOCOL, " ",
>  ap_get_status_line(100), CRLF CRLF, NULL);

Notably form_header_field() etc will DTRT on EBCDIC platforms, which the 
current code does not - I agree with Ruediger that http_filters.c is the 
right place for this function.

...
> I think http_filters.c is dealing with automatic HTTP negotiation
> controlled by the core, whereas ap_send_interim_response is an
> API for applications (including proxy) and hence belongs in
> protocol.c.  Unless you can convince me otherwise???

ap_*_client_block(), ap_discard_request_body() are all part of the 
request-handling API and live in http_filters.c, this isn't much 
different in nature.

joe

Re: svn commit: r583829 - /httpd/httpd/branches/2.2.x/STATUS

[Ruediger Pluem]

On 10/11/2007 04:12 PM, [EMAIL PROTECTED] wrote:
> Author: jim
> Date: Thu Oct 11 07:12:02 2007
> New Revision: 583829
> 
> URL: http://svn.apache.org/viewvc?rev=583829&view=rev
> Log:
> Add fix for, as of now, unconfirmed issue...
> 
> Modified:
> httpd/httpd/branches/2.2.x/STATUS
> 
> Modified: httpd/httpd/branches/2.2.x/STATUS
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=583829&r1=583828&r2=583829&view=diff
> ==
> --- httpd/httpd/branches/2.2.x/STATUS (original)
> +++ httpd/httpd/branches/2.2.x/STATUS Thu Oct 11 07:12:02 2007
> @@ -207,6 +207,12 @@
>   http://svn.apache.org/viewvc?view=rev&revision=582631
>   +1: niq
>  
> +* modules/proxy/mod_proxy_http.c (ap_proxy_http_register_hook): Fix
> +  apr_pool_cleanup_register() invocation added in r583202, which was
> +  causing every apr_proc_create() call to segfault.
> +  Trunk version of patch:
> +http://svn.apache.org/viewvc?view=rev&revision=583813
> +

I assume you are +1 on your proposal, right :-)?

Regards

Rüdiger

thoughts on ETags and mod_dav

[Chris Darroch]

Hi --

   A couple of months ago a short thread started in relation to the
PRs #16593 and #38034 (which also references #42987) on the various
problems related to ETags:

http://marc.info/?l=apache-httpd-dev&m=118831732512678&w=2

http://issues.apache.org/bugzilla/show_bug.cgi?id=16593
http://issues.apache.org/bugzilla/show_bug.cgi?id=38034
http://issues.apache.org/bugzilla/show_bug.cgi?id=42987

   I'm not about to tackle these all immediately, but I wanted to
send out a couple of proposals and find out what problems folks
might see with them.


1) Per #38034, it appears that ap_meets_conditions() treats "*" incorrectly.
The patch attached to the PR duplicates ap_meets_conditions() for
exclusive use by mod_dav.  The only changes are to add checks on
resource->exists in two places and to use resource->hooks->getetag instead
of looking for an ETag header in r->headers_out.

   In the interest of avoiding code duplication, I wonder if it would
be possible to continue to use ap_meets_conditions() by using
r->no_local_copy in place of resource->exists, and by setting/unsetting
that and an ETag header in r->headers_out in mod_dav as necessary;
e.g., prior to calls to ap_meets_conditions().

   However, it looks to me as though r->no_local_copy is used in a few
places to bypass the ap_meets_conditions() checks entirely (i.e., to
never send a 304 response in certain cases).

   Does anyone have comments on what the intended purpose
r->no_local_copy is or was, especially in relation to the language
of RFC 2616 sections 14.24 and 14.26 ("if '*' is given and any current
entity exists for that resource ...")?

   Would it be more appropriate and/or required by our backporting
rules to add another field to request_rec, like r->exists, to indicate
to ap_meets_conditions() that a resource does not exist?  Or to use
r->notes or conceivably r->finfo for the same purpose?

   Is it even possible to backport a change to the action of
ap_meets_conditions() such that it would start depending on data
which third-party callers won't be supplying?  Advice is welcome, please!


2) I haven't looked at #16593 in any detail, but I wonder if the
change proposed in the patch attached to #38034 where
resource->hooks->getetag is used instead of looking for an ETag header
in r->headers_out might not be an attempt to deal with this issue.
If so, then whatever the eventual solution for #38034 turns out to
be will likely resolve this PR as well.


3) Per #42987, it seems as if there's a subtle difference between
the requirements for ETags as described in RFC 2616 section 13.3.3
and the way we currently generate them in ap_make_etag().

   When a file's mtime is in the past, ap_make_etag() uses the file's
inode, length, and/or mtime to create a strong ETag.  However, when the
mtime is the same as r->request_time, a weak ETag is generated.  As the
attachment to the PR reveals, though, this isn't really sufficient when
certain sequences of requests occur within a single second.

   The RFC says that a file's mtime "could" be a weak validator, because
the file might change more than once in a second.  But I think the
subtly important issue here is that word "could".  If I understand it
correctly, the implication is that one has resource for which one wants
to generate weak ETags only, because one knows the semantic meaning isn't
changing although the content may be.  In this particular case, the
modification time *might* be appropriate to use as a weak validator
(although that seems somewhat unlikely, because non-semantic changes
that occurred seconds apart would still cause different weak ETags to be
generated, which probably isn't what you'd want in this circumstance).

   But, in the cases handled by ap_make_etag(), the content and/or the
semantic meaning of a resource could always be changing, so weak ETags
would seem to be, as the PR says, simply never appropriate.

   Alas, though, there is probably no way to quickly generate a strong
validators like a hash -- which is why we're using inodes, lengths, and
mtimes in the first place, of course.

   So, a modest proposal, which aims to punt the problem to the
administrator, and also to resolve the fact that FileETag is provided
to configure the actions of ap_make_etag(), while mod_dav_fs's
dav_fs_getetag() actions are fixed.  The highlights would be:

- refactor ap_make_etag() and dav_fs_getetag() to use a common
  code base as far as possible

- make dav_fs_getetag() follow the options set by FileETag (if this
  is a problem, then introduce a DavFileETag directive, but I'm hoping
  that could be avoided)

- add MD5 and SHA1 as options to FileETag (while obviously much
  slower, certain applications may require "really strong" validators);
  for legacy reasons, these would have to be specifically configured
  and would not be implied by the "All" option

- add a nomtime=none|weak|ignore option to FileETag (in the manner
  of ProxyPass) which specifies what to do when the MTime option is
  co

Re: thoughts on ETags and mod_dav

[Paritosh Shah]

Hi,

There is a way we can avoid code duplication of ap_meets_conditions()
- handle the cases that ap_meets_conditions() does not handle
separately. For this, we can create a new dav_meets_conditions() which
calls ap_meets_conditions() and also handles those other cases. Also,
the problem of ETag header not being set, can be tackled by calling
the resource->hook->set_headers() hook before calling
dav_meets_conditions().

Attaching a patch containing these changes. The patch is against
apache v2.2.6. And I've tested it against mod_dav_fs ( w/
DEBUG_GET_HANDLER set to 1) for the following cases:
1. IF-MATCH * with nothing there fails
2. IF-NONE-MATCH * with nothing there succeeds
3. IF-NONE-MATCH * with something there fails
4. IF-MATCH * with something there succeeds
5. IF-MATCH with same etag succeeds
6. IF-MATCH with different etag fails

For test case 5. you've to be careful about the weak etag bug ( sleep
for a couple of seconds between consecutive requests )

- Paritosh.

On 10/11/07, Chris Darroch <[EMAIL PROTECTED]> wrote:
> Hi --
>
>A couple of months ago a short thread started in relation to the
> PRs #16593 and #38034 (which also references #42987) on the various
> problems related to ETags:
>
> http://marc.info/?l=apache-httpd-dev&m=118831732512678&w=2
>
> http://issues.apache.org/bugzilla/show_bug.cgi?id=16593
> http://issues.apache.org/bugzilla/show_bug.cgi?id=38034
> http://issues.apache.org/bugzilla/show_bug.cgi?id=42987
>
>I'm not about to tackle these all immediately, but I wanted to
> send out a couple of proposals and find out what problems folks
> might see with them.
>
>
> 1) Per #38034, it appears that ap_meets_conditions() treats "*" incorrectly.
> The patch attached to the PR duplicates ap_meets_conditions() for
> exclusive use by mod_dav.  The only changes are to add checks on
> resource->exists in two places and to use resource->hooks->getetag instead
> of looking for an ETag header in r->headers_out.
>
>In the interest of avoiding code duplication, I wonder if it would
> be possible to continue to use ap_meets_conditions() by using
> r->no_local_copy in place of resource->exists, and by setting/unsetting
> that and an ETag header in r->headers_out in mod_dav as necessary;
> e.g., prior to calls to ap_meets_conditions().
>
>However, it looks to me as though r->no_local_copy is used in a few
> places to bypass the ap_meets_conditions() checks entirely (i.e., to
> never send a 304 response in certain cases).
>
>Does anyone have comments on what the intended purpose
> r->no_local_copy is or was, especially in relation to the language
> of RFC 2616 sections 14.24 and 14.26 ("if '*' is given and any current
> entity exists for that resource ...")?
>
>Would it be more appropriate and/or required by our backporting
> rules to add another field to request_rec, like r->exists, to indicate
> to ap_meets_conditions() that a resource does not exist?  Or to use
> r->notes or conceivably r->finfo for the same purpose?
>
>Is it even possible to backport a change to the action of
> ap_meets_conditions() such that it would start depending on data
> which third-party callers won't be supplying?  Advice is welcome, please!
>
>
> 2) I haven't looked at #16593 in any detail, but I wonder if the
> change proposed in the patch attached to #38034 where
> resource->hooks->getetag is used instead of looking for an ETag header
> in r->headers_out might not be an attempt to deal with this issue.
> If so, then whatever the eventual solution for #38034 turns out to
> be will likely resolve this PR as well.
>
>
> 3) Per #42987, it seems as if there's a subtle difference between
> the requirements for ETags as described in RFC 2616 section 13.3.3
> and the way we currently generate them in ap_make_etag().
>
>When a file's mtime is in the past, ap_make_etag() uses the file's
> inode, length, and/or mtime to create a strong ETag.  However, when the
> mtime is the same as r->request_time, a weak ETag is generated.  As the
> attachment to the PR reveals, though, this isn't really sufficient when
> certain sequences of requests occur within a single second.
>
>The RFC says that a file's mtime "could" be a weak validator, because
> the file might change more than once in a second.  But I think the
> subtly important issue here is that word "could".  If I understand it
> correctly, the implication is that one has resource for which one wants
> to generate weak ETags only, because one knows the semantic meaning isn't
> changing although the content may be.  In this particular case, the
> modification time *might* be appropriate to use as a weak validator
> (although that seems somewhat unlikely, because non-semantic changes
> that occurred seconds apart would still cause different weak ETags to be
> generated, which probably isn't what you'd want in this circumstance).
>
>But, in the cases handled by ap_make_etag(), the content and/or the
> semantic meaning of a resource could alwa

Re: thoughts on ETags and mod_dav

[Chris Darroch]

Hi --

> 1) Per #38034, it appears that ap_meets_conditions() treats "*" incorrectly.

   More precisely, I should say that ap_meets_conditions() isn't designed
to support the NULL resources of RFC 2518 (WebDAV).  I'm certainly no
expert on these issues, so guidance is welcome.

   RFC 2616 section 14.24 (and 14.26 is similar) says, "If the request
would, without the If-Match header field, result in anything other than a
2xx or 412 status, then the If-Match header MUST be ignored."  Thus in
the typical case, if a resource doesn't exist, 404 should be returned,
so ap_meets_conditions() doesn't need to handle this case at all.

   RFC 2518 introduces lock-null resources for use with certain
methods such as LOCK, PUT, and UNLOCK.  (RFC 4918, it seems, deprecates
lock-null resources in favour of "locked empty resources".)  For these
particular methods and resources, other language from RFC 2616's
section 14.24 (again, 14.26 is similar) comes into play, specifically,
"If none of the entity tags match, or if '*' is given and no current entity
exists, the server MUST NOT perform the requested method, and MUST return a
412 (Precondition Failed) response."

   Personally, I'd be inclined to try to make ap_meets_conditions()
handle support these different situations in as generic a way as
possible (in case other non-mod_dav DAV modules or other HTTP extensions
need to use the same logic), but that may be difficult to do without
introducing compatibility problems and/or contorted code.

   Another option would be the one described by Paritosh Shah:

> For this, we can create a new dav_meets_conditions() which
> calls ap_meets_conditions() and also handles those other cases.

   Thoughts, corrections, etc.?

Chris.

-- 
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263  E4DE C8E3 FA36 366A 375B