mod_cache: Broken code?
When trying to debug something else I stumbled across this
code-snippet in modules/cache/mod_cache.c:
errno = 0;
x = control.max_age_value;
if (errno) {
x = dconf->defex;
}
else {
x = x * MSEC_ONE_SEC;
}
It looks that way both in trunk and 2.4.x.
The likelhood of that if-statement to have more than one outcome is
low, and the fact that errno isn't used anywhere else in mod_cache.c
makes it even more suspicious.
Looking at the annotated history it seems like the errno-stuff stems
from apr_atoi64() being used once upon a time but has since been
removed without cleaning up the related code...
It's easy to just remove the now redundant code, but is that the right
way to do it or did the initial code have some function that's now
gone missing?
/Nikke
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se | ni...@acc.umu.se
---
Those who can, do. Those who can't, simulate.
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Re: Version check idea
Le 21/04/2015 15:55, Jim Jagielski a écrit : For comment: What do people think about adding the capability that when httpd is started, it tries to access http://httpd.apache.org/doap.rdf to check its version number with the latest one referred to in that file and, if a newer one exists, it prints out a little message in the error.log (or stderr)... My 2 cents. I don't really see the use to have such an info at start up. I would expect web server not to restart that often. As said elsewhere, I suppose that server version that are running are the ones from the distribution used. So most user will not care about new version. Moreover, in the case of a own build server, I expect that these users know what they do and where they can easily find information about the latest version, including sources in order to update. in other words, I don't the use in real world, in any case. However, other options could be: (going, from my point of view, from worst to better) - Send at startup "some data" to a server, including email of someone. This remote server would store these data, make the check and send by email if an upgrade exists. Knowing the version of the remote web server when it was last started could be used to make regular check. In such a case we would also need to uniquely identify all theses web server. These "data" could also include loaded modules in order to have some information on what is most used. (if of any interest) - Provide a service somewhere where users can register and define which apache foundation softwares (why limit to httpd?) they are running, define which version they are using. They would be notified by email when new version are released. - Add options in mod_status to make the check. (where to check, how often (once a day, once a week...)) Each access to mod_status would this way display the latest version and if up-to-date - Only add a link in the page generated by mod_status. This link could be by default something like : http://https.apache.org/check_version.lua?CurVersion=2.4.1 This would display a page which would check the differences between the running version and the current version or say that it is up-to-date Best regards, CJ
Re: Extending mod_authz_dbd
Hi Graham,
Here's a first patch that adds the functionality that you suggested.
> Require dbd-login %{REQUEST_URI} %{REQUEST_METHOD} %{REMOTE_USER}
> AuthzDBDQuery "UPDATE authn SET uri = %s, method = %s WHERE user = %s”
>
> Require dbd-logout %{TIME} %{REMOTE_USER}
> AuthzDBDQuery "UPDATE authn SET logout_time = %s WHERE user = %s”
>
> To be backwards compatible, "Require dbd-login” on it’s own would imply
> "Require dbd-login %{REMOTER_USER}”.
>
> One possible approach to support completely generic queries might be as
> follows:
>
> Require dbd-query %{REQUEST_URI} %{REMOTE_USER}
> AuthzDBDQuery “SELECT count(user) FROM authn WHERE uri=%s AND user = %s”
Although I developed and tested this against the 2.4.10 source
tree (the latest debian jessie version), I checked and this
module as well as mod_dbd.c have not changed since then neither
in the latest 2.4.x nor in trunk. I don't think there will be
an issue running it there. Installing that environment over the
default debian one could be a hazzle for me that I'd prefer
to avoid , but that I could do if needed.
Here below are some development notes.
Please keep me update if I can contribute something else to this
patch, that it be documentation or further changes. I understand
there's no engagement to accept this code from you. We can chat
on irc too if needed.
Cheers,
-jose
Open issues:
- I wanted to control that the number of bind arguments correspond to
those in the prepared request. However this value is not visible in
apr_dbd_prepared_t *query. Looking at the code in apr_dbd.c, I see
that even though the number of bind arguments are being passed to
apr_dbd_pselect() and apr_dbd_pquery() in nargs, this argument is
discarded before calling the implementation of those functions in
the driver.
At least in the mysql driver, there is no further check that there
is an equivalence between the bind and prepared statement
arguments. In both cases there is just a call to
mysql_stmt_bind_param(statement->stmt, bind);
which expects bind to have the correct number of arguments. I
checked the sqlite3 driver and it has the same issue.
If bind has more arguments, there is no issue. If there is less, we
risk a SIGSEV or unexpected behavior.
To avoid this issue, I wanted to control the number of arguments
when parsing the configuration file and show a configuration error
when necessary. However, as the prepared query goes into opaque
driver implementations, I don't have access to its nargs value. Are
there any ideas on how to get this info? The only hack I could
think of is counting the number of % inside the prepared query and
making sure that we have the corresponding number of bind arguments.
- Would it be worth it to connect this module to mod_socache.c?
Other questions:
- In function dbd_parse_config, I was unable to decide whether it
would be ok to use cmd->temp_pool instead of cmd->pool in my call to
ap_getword_white()
- I am using ap_expr_parse_cmd() on each argument of the require
dbd-query, dbd-login, and dbd-logout directives. I think this is
akin to building an expression tree for each argument. Is there a
more efficient way to do so?
Index: mod_authz_dbd.c
===
--- mod_authz_dbd.c (revision 1675706)
+++ mod_authz_dbd.c (working copy)
@@ -109,7 +109,45 @@
{NULL}
};
+static int evaluate_query_parameters(request_rec *r,
+ const apr_array_header_t *parsed_require_args,
+ const void **query_parameters)
+{
+int i;
+apr_array_header_t *qp;
+
+const ap_expr_info_t *expr = NULL;
+const char *parameter;
+
+const char *err = NULL;
+
+/* evaluate the query parameters in parsed_require_args */
+qp = apr_array_make(r->pool,
+ parsed_require_args->nelts,
+ sizeof (char *));
+
+for (i = 0; i < parsed_require_args->nelts; i++) {
+
+expr = ((const ap_expr_info_t **)parsed_require_args->elts)[i];
+parameter = ap_expr_str_exec(r, expr, &err);
+
+if (err) {
+ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO()
+ "authz_dbd in evaluate query_parameters: Can't "
+ "evaluate require expression: %s", err);
+return HTTP_INTERNAL_SERVER_ERROR;
+}
+
+*(const char **)apr_array_push(qp) = parameter;
+}
+
+*query_parameters = (void *)qp;
+
+return OK;
+}
+
static int authz_dbd_login(request_rec *r, authz_dbd_cfg *cfg,
+ const void *parsed_require_args,
const char *action)
{
int rv;
@@ -120,12 +158,18 @@
apr_dbd_prepared_t *query;
apr_dbd_results_t *res = NULL;
apr_dbd_row_t *row = NULL;
+apr_array_header_t *query_parameters;
if (cfg->query == NULL) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01
Re: trunk and FreeBSD 10.1
I tried that, but gmake totally barfed... > On Apr 23, 2015, at 12:07 PM, Jeff Trawick wrote: > > On 04/23/2015 11:59 AM, Jim Jagielski wrote: >> Hmmm... I am seeing some strangeness trying to get trunk to >> build on FreeBSD 10.1... I get to ./server/ and then the >> build dies w/ > > try gmake? > > IIRC I had noticed that some BSD make compatibility was lost at some point... >> >>-DCROSS_COMPILE -o gen_test_char >>make[2]: exec(-DCROSS_COMPILE) failed (No such file or directory) >>*** Error code 1 >> >> It looks like the Makefile.in->Makefile process is causing issues. >> This is what I get in the Makefile: >> >> >>.include "$(top_builddir)/build/rules.mk" >>.include "$(top_srcdir)/build/library.mk" >> >>.ifdef CC_FOR_BUILD >>gen_test_char: gen_test_char.c >> $(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) -DCROSS_COMPILE -o $@ $< >>.else >>gen_test_char_OBJECTS = gen_test_char.lo >>gen_test_char: $(gen_test_char_OBJECTS) >> $(LINK) $(EXTRA_LDFLAGS) $(gen_test_char_OBJECTS) $(EXTRA_LIBS) >>.endif >> >> So we can see where the error is... those shell vars aren't >> defined/found. But why? >
Re: Version check idea
On Apr 23, 2015, at 7:50 AM, dev-digest-h...@httpd.apache.org wrote: > > From: Jim Jagielski > Subject: Version check idea > Date: April 21, 2015 at 8:55:38 AM CDT > To: httpd > > > For comment: What do people think about adding the capability that > when httpd is started, it tries to access http://httpd.apache.org/doap.rdf > to check its version number with the latest one referred to in that > file and, if a newer one exists, it prints out a little message > in the error.log (or stderr)... > I’m sure this has been said, but I don’t see a reason for this feature. 1. Most distributions purposely pin their Apache to older versions, then back-port their fixes into it. So they’re always going to be out-of-date. 2. I don’t see this as a necessity of a web server. If this feature is necessary (for those that build their own stacks), why not bundle a separate utility that performs the check. 3. This feature wouldn’t be terrible useful since the goal of a web server is to stay up. Checking at start-up seems counter-intuitive.
Re: Version check idea
Hmmm... I wonder if the config directive could take a URL as a parameter as an alternative... > On Apr 23, 2015, at 8:48 AM, Rich Bowen wrote: > > > > On 04/21/2015 10:13 AM, Jeff Trawick wrote: >> On 04/21/2015 09:55 AM, Jim Jagielski wrote: >>> For comment: What do people think about adding the capability that >>> when httpd is started, it tries to access >>> http://httpd.apache.org/doap.rdf >>> to check its version number with the latest one referred to in that >>> file and, if a newer one exists, it prints out a little message >>> in the error.log (or stderr)... >>> >> Wild assertion: Most people don't get their httpd build from us, so it >> will be meaningless for them (i.e., says out-of-date the first day they >> fire up their shiny new Fedora-latest/Ubuntu-latest/etc.). >> > > Provide either a configuration option, or other easy way for packagers to > alter where ET phones home, to compensate for that reality. > > --Rich > > -- > Rich Bowen - rbo...@rcbowen.com - @rbowen > http://apachecon.com/ - @apachecon
Re: trunk and FreeBSD 10.1
On 04/23/2015 11:59 AM, Jim Jagielski wrote: Hmmm... I am seeing some strangeness trying to get trunk to build on FreeBSD 10.1... I get to ./server/ and then the build dies w/ try gmake? IIRC I had noticed that some BSD make compatibility was lost at some point... -DCROSS_COMPILE -o gen_test_char make[2]: exec(-DCROSS_COMPILE) failed (No such file or directory) *** Error code 1 It looks like the Makefile.in->Makefile process is causing issues. This is what I get in the Makefile: .include "$(top_builddir)/build/rules.mk" .include "$(top_srcdir)/build/library.mk" .ifdef CC_FOR_BUILD gen_test_char: gen_test_char.c $(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) -DCROSS_COMPILE -o $@ $< .else gen_test_char_OBJECTS = gen_test_char.lo gen_test_char: $(gen_test_char_OBJECTS) $(LINK) $(EXTRA_LDFLAGS) $(gen_test_char_OBJECTS) $(EXTRA_LIBS) .endif So we can see where the error is... those shell vars aren't defined/found. But why?
trunk and FreeBSD 10.1
Hmmm... I am seeing some strangeness trying to get trunk to build on FreeBSD 10.1... I get to ./server/ and then the build dies w/ -DCROSS_COMPILE -o gen_test_char make[2]: exec(-DCROSS_COMPILE) failed (No such file or directory) *** Error code 1 It looks like the Makefile.in->Makefile process is causing issues. This is what I get in the Makefile: .include "$(top_builddir)/build/rules.mk" .include "$(top_srcdir)/build/library.mk" .ifdef CC_FOR_BUILD gen_test_char: gen_test_char.c $(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) -DCROSS_COMPILE -o $@ $< .else gen_test_char_OBJECTS = gen_test_char.lo gen_test_char: $(gen_test_char_OBJECTS) $(LINK) $(EXTRA_LDFLAGS) $(gen_test_char_OBJECTS) $(EXTRA_LIBS) .endif So we can see where the error is... those shell vars aren't defined/found. But why?
Re: Apache (httpd) Wiki
On Apr 23, 2015 7:54 AM, "Rich Bowen" wrote: > > > > On 04/22/2015 10:53 AM, Tom Browder wrote: >> The error phrase is here: >> >> "set all files to 640, or rw-r--r--" >> >> which should read: >> >> "set all files to 640, or rw-r-" > > Thanks. Fixed. Thanks for a rapid fix, Rich. I haven't tried adding a new page at the same place as the file permissions page. Is that possible for an ordinary user? I would like to contribute my successful method for using the latest openssl with the latest Apache while not interering with the system openssl (from Ivan Ristic with slight mods). Best regards, -Tom
Re: Apache (httpd) Wiki
On 04/22/2015 10:53 AM, Tom Browder wrote: There is an error on this page which is "immutable" and cannot be edited by an ordinary user (even logged in): https://wiki.apache.org/httpd/FileSystemPermissions The error is in this the last line: What we've done here is to set all files to 640, or rw-r--r-- and directories to rwxr-x---. Because the group "web-content" is applied to all the files and directories, httpd can read these files, but cannot write to them. The error phrase is here: "set all files to 640, or rw-r--r--" which should read: "set all files to 640, or rw-r-" Thanks. Fixed. -- Rich Bowen - rbo...@rcbowen.com - @rbowen http://apachecon.com/ - @apachecon
Re: Version check idea
On 04/21/2015 10:13 AM, Jeff Trawick wrote: On 04/21/2015 09:55 AM, Jim Jagielski wrote: For comment: What do people think about adding the capability that when httpd is started, it tries to access http://httpd.apache.org/doap.rdf to check its version number with the latest one referred to in that file and, if a newer one exists, it prints out a little message in the error.log (or stderr)... Wild assertion: Most people don't get their httpd build from us, so it will be meaningless for them (i.e., says out-of-date the first day they fire up their shiny new Fedora-latest/Ubuntu-latest/etc.). Provide either a configuration option, or other easy way for packagers to alter where ET phones home, to compensate for that reality. --Rich -- Rich Bowen - rbo...@rcbowen.com - @rbowen http://apachecon.com/ - @apachecon
Re: ALPN patch comments
Am 23.04.2015 um 06:56 schrieb Kaspar Brand: On 22.04.2015 21:30, Rainer Jung wrote: Am 22.04.2015 um 17:49 schrieb Kaspar Brand: Thanks. In the patch for ssl_private.h, the complete NPN block should actually be dropped - the same block is are already part of ssl_private.h, just 10 lines above. I've kept the new one and dropped the old one in r1675459. It's nitpicking, but... the old one was already correct, actually. All this stuff is in a larger "#if !defined(OPENSSL_NO_TLSEXT) ..." block, so there's no need to repeat NO_TLSEXT. Oups, fixed in r1675549. I'd rather get consensus on completely dropping HAVE_TLS_NPN from trunk anyway, though, so it's really a very minor point right now ;-) Agreed. Rainer
