Re: How to align shm in an neat way?
On 13.08.2012 21:02, Rainer Jung wrote: On 13.08.2012 19:40, Jeff Trawick wrote: On Mon, Aug 13, 2012 at 1:27 PM, Rainer Jung wrote: On 13.08.2012 18:32, Jeff Trawick wrote: On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung wrote: Hi, PR 53040 reveals, that mod_socache_shmcb has an alignment problem. One of the three structs mapped into shm contains an apr_time_t member, which at least on Sparc is 8 Bytes, whereas for 32 bit builds long is only 4 Bytes. Currently everything is aligned for 4 Bytes, so we get bus errors/crashes when trying to assign the apr_time_t to an address that is only divisible by 4 instead of 8. I can easily reproduce the problem. A possible solution is to pad the three structures SHMCBHeader, SHMCBSubcache and SHMCBIndex to a multiple of 8 Bytes length. For Subcache and Index this is already true by coincidence, SHMCBHeader needs another 4 Bytes. I wonder what the right solution is. In the patch http://people.apache.org/~rjung/patches/mod_socache_shmcb-padding.patch I hard coded the padding, but I don't really like it, because it breaks if members are added to the struct. I could add a sizeof() test during startup or probably even compilation to warn or err, if the padding is wrong. I see several recipes for alignment using pragmas and attribute, but all of them are compiler specific. One could also wrap the struct in a wrapped struct, so that one could use the sizeof() of the inner struct to determine the padding of the outer struct. That would make the code convoluted. I checked other parts of the code, but couldn't find a simple solution. Any hints how to do this nicely? APR_ALIGN_DEFAULT? I think it doesn't solve this problem, does it? It only gives me a need way to round up sizes to multiples of 8 bytes. It should be possible to use use APR_ALIGN_DEFAULT(sizeof(foo)) in place of sizeof(foo), instead of adding explicit padding to the structures. Any other pointer arithmetic which doesn't use sizeof(foo) would also need to use the macro. Understood. Unfortunately currently the code doesn't really care about alignment. So it just puts structs SHMCBHeader, SHMCBSubcache and an array of SHMCBIndex directly after each other in shm. No sizeof() involved. So yes, option 3 would be to rewrite the code to calculate the gaps/padding between the structs using APR_ALIGN_DEFAULT() and adjust memory alignment in shm. I wanted to get around changing that part of the code ;) I went the "choose right alignment" way now: http://people.apache.org/~rjung/patches/mod_socache_shmcb-alignment.patch It actually wasn't that complicated. Regards, Rainer
Re: How to align shm in an neat way?
On 13.08.2012 19:40, Jeff Trawick wrote: On Mon, Aug 13, 2012 at 1:27 PM, Rainer Jung wrote: On 13.08.2012 18:32, Jeff Trawick wrote: On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung wrote: Hi, PR 53040 reveals, that mod_socache_shmcb has an alignment problem. One of the three structs mapped into shm contains an apr_time_t member, which at least on Sparc is 8 Bytes, whereas for 32 bit builds long is only 4 Bytes. Currently everything is aligned for 4 Bytes, so we get bus errors/crashes when trying to assign the apr_time_t to an address that is only divisible by 4 instead of 8. I can easily reproduce the problem. A possible solution is to pad the three structures SHMCBHeader, SHMCBSubcache and SHMCBIndex to a multiple of 8 Bytes length. For Subcache and Index this is already true by coincidence, SHMCBHeader needs another 4 Bytes. I wonder what the right solution is. In the patch http://people.apache.org/~rjung/patches/mod_socache_shmcb-padding.patch I hard coded the padding, but I don't really like it, because it breaks if members are added to the struct. I could add a sizeof() test during startup or probably even compilation to warn or err, if the padding is wrong. I see several recipes for alignment using pragmas and attribute, but all of them are compiler specific. One could also wrap the struct in a wrapped struct, so that one could use the sizeof() of the inner struct to determine the padding of the outer struct. That would make the code convoluted. I checked other parts of the code, but couldn't find a simple solution. Any hints how to do this nicely? APR_ALIGN_DEFAULT? I think it doesn't solve this problem, does it? It only gives me a need way to round up sizes to multiples of 8 bytes. It should be possible to use use APR_ALIGN_DEFAULT(sizeof(foo)) in place of sizeof(foo), instead of adding explicit padding to the structures. Any other pointer arithmetic which doesn't use sizeof(foo) would also need to use the macro. Understood. Unfortunately currently the code doesn't really care about alignment. So it just puts structs SHMCBHeader, SHMCBSubcache and an array of SHMCBIndex directly after each other in shm. No sizeof() involved. So yes, option 3 would be to rewrite the code to calculate the gaps/padding between the structs using APR_ALIGN_DEFAULT() and adjust memory alignment in shm. I wanted to get around changing that part of the code ;) Regards, Rainer
Re: How to align shm in an neat way?
On Mon, Aug 13, 2012 at 1:27 PM, Rainer Jung wrote: > On 13.08.2012 18:32, Jeff Trawick wrote: >> >> On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung >> wrote: >>> >>> Hi, >>> >>> PR 53040 reveals, that mod_socache_shmcb has an alignment problem. One of >>> the three structs mapped into shm contains an apr_time_t member, which at >>> least on Sparc is 8 Bytes, whereas for 32 bit builds long is only 4 >>> Bytes. >>> >>> Currently everything is aligned for 4 Bytes, so we get bus errors/crashes >>> when trying to assign the apr_time_t to an address that is only divisible >>> by >>> 4 instead of 8. >>> >>> I can easily reproduce the problem. >>> >>> A possible solution is to pad the three structures SHMCBHeader, >>> SHMCBSubcache and SHMCBIndex to a multiple of 8 Bytes length. For >>> Subcache >>> and Index this is already true by coincidence, SHMCBHeader needs another >>> 4 >>> Bytes. >>> >>> I wonder what the right solution is. In the patch >>> >>> http://people.apache.org/~rjung/patches/mod_socache_shmcb-padding.patch >>> >>> I hard coded the padding, but I don't really like it, because it breaks >>> if >>> members are added to the struct. I could add a sizeof() test during >>> startup >>> or probably even compilation to warn or err, if the padding is wrong. >>> >>> I see several recipes for alignment using pragmas and attribute, but all >>> of >>> them are compiler specific. >>> >>> One could also wrap the struct in a wrapped struct, so that one could use >>> the sizeof() of the inner struct to determine the padding of the outer >>> struct. That would make the code convoluted. >>> >>> I checked other parts of the code, but couldn't find a simple solution. >>> Any >>> hints how to do this nicely? >> >> >> APR_ALIGN_DEFAULT? > > > I think it doesn't solve this problem, does it? It only gives me a need way > to round up sizes to multiples of 8 bytes. It should be possible to use use APR_ALIGN_DEFAULT(sizeof(foo)) in place of sizeof(foo), instead of adding explicit padding to the structures. Any other pointer arithmetic which doesn't use sizeof(foo) would also need to use the macro. > > Regards, > > Rainer > -- Born in Roswell... married an alien... http://emptyhammock.com/
Re: How to align shm in an neat way?
On 13.08.2012 18:32, Jeff Trawick wrote: On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung wrote: Hi, PR 53040 reveals, that mod_socache_shmcb has an alignment problem. One of the three structs mapped into shm contains an apr_time_t member, which at least on Sparc is 8 Bytes, whereas for 32 bit builds long is only 4 Bytes. Currently everything is aligned for 4 Bytes, so we get bus errors/crashes when trying to assign the apr_time_t to an address that is only divisible by 4 instead of 8. I can easily reproduce the problem. A possible solution is to pad the three structures SHMCBHeader, SHMCBSubcache and SHMCBIndex to a multiple of 8 Bytes length. For Subcache and Index this is already true by coincidence, SHMCBHeader needs another 4 Bytes. I wonder what the right solution is. In the patch http://people.apache.org/~rjung/patches/mod_socache_shmcb-padding.patch I hard coded the padding, but I don't really like it, because it breaks if members are added to the struct. I could add a sizeof() test during startup or probably even compilation to warn or err, if the padding is wrong. I see several recipes for alignment using pragmas and attribute, but all of them are compiler specific. One could also wrap the struct in a wrapped struct, so that one could use the sizeof() of the inner struct to determine the padding of the outer struct. That would make the code convoluted. I checked other parts of the code, but couldn't find a simple solution. Any hints how to do this nicely? APR_ALIGN_DEFAULT? I think it doesn't solve this problem, does it? It only gives me a need way to round up sizes to multiples of 8 bytes. Regards, Rainer
Re: How to align shm in an neat way?
On Mon, Aug 13, 2012 at 12:30 PM, Rainer Jung wrote: > Hi, > > PR 53040 reveals, that mod_socache_shmcb has an alignment problem. One of > the three structs mapped into shm contains an apr_time_t member, which at > least on Sparc is 8 Bytes, whereas for 32 bit builds long is only 4 Bytes. > > Currently everything is aligned for 4 Bytes, so we get bus errors/crashes > when trying to assign the apr_time_t to an address that is only divisible by > 4 instead of 8. > > I can easily reproduce the problem. > > A possible solution is to pad the three structures SHMCBHeader, > SHMCBSubcache and SHMCBIndex to a multiple of 8 Bytes length. For Subcache > and Index this is already true by coincidence, SHMCBHeader needs another 4 > Bytes. > > I wonder what the right solution is. In the patch > > http://people.apache.org/~rjung/patches/mod_socache_shmcb-padding.patch > > I hard coded the padding, but I don't really like it, because it breaks if > members are added to the struct. I could add a sizeof() test during startup > or probably even compilation to warn or err, if the padding is wrong. > > I see several recipes for alignment using pragmas and attribute, but all of > them are compiler specific. > > One could also wrap the struct in a wrapped struct, so that one could use > the sizeof() of the inner struct to determine the padding of the outer > struct. That would make the code convoluted. > > I checked other parts of the code, but couldn't find a simple solution. Any > hints how to do this nicely? APR_ALIGN_DEFAULT? > > Regards, > > Rainer -- Born in Roswell... married an alien... http://emptyhammock.com/
How to align shm in an neat way?
Hi, PR 53040 reveals, that mod_socache_shmcb has an alignment problem. One of the three structs mapped into shm contains an apr_time_t member, which at least on Sparc is 8 Bytes, whereas for 32 bit builds long is only 4 Bytes. Currently everything is aligned for 4 Bytes, so we get bus errors/crashes when trying to assign the apr_time_t to an address that is only divisible by 4 instead of 8. I can easily reproduce the problem. A possible solution is to pad the three structures SHMCBHeader, SHMCBSubcache and SHMCBIndex to a multiple of 8 Bytes length. For Subcache and Index this is already true by coincidence, SHMCBHeader needs another 4 Bytes. I wonder what the right solution is. In the patch http://people.apache.org/~rjung/patches/mod_socache_shmcb-padding.patch I hard coded the padding, but I don't really like it, because it breaks if members are added to the struct. I could add a sizeof() test during startup or probably even compilation to warn or err, if the padding is wrong. I see several recipes for alignment using pragmas and attribute, but all of them are compiler specific. One could also wrap the struct in a wrapped struct, so that one could use the sizeof() of the inner struct to determine the padding of the outer struct. That would make the code convoluted. I checked other parts of the code, but couldn't find a simple solution. Any hints how to do this nicely? Regards, Rainer
Re: core filters vs non-blocking socket (was Re: Fix for Windows bug#52476)
On Mon, Aug 13, 2012 at 09:27:08AM -0400, Jeff Trawick wrote: > Does that explanation work for you? Yes, perfectly, thanks for taking the time. I stupidly forgot about the timeout calls... sorry! Regards, Joe
Re: Fix for Windows bug#52476
On Mon, Aug 13, 2012 at 8:58 AM, Apache Lounge wrote: > Also here it is running now without issues till now here with > AcceptFilter-none+SSL awesome/thanks! > > Steffen > > -Original Message- From: Jeff Trawick > Sent: Friday, August 10, 2012 7:43 PM Newsgroups: gmane.comp.apache.devel > To: dev@httpd.apache.org > Subject: Re: Fix for Windows bug#52476 > > > This patch is testing great so far with the AcceptFilter-none+SSL > scenario on Windows. > > Index: server/core_filters.c > === > --- server/core_filters.c (revision 1371377) > +++ server/core_filters.c (working copy) > @@ -391,10 +391,6 @@ > if (ctx == NULL) { > ctx = apr_pcalloc(c->pool, sizeof(*ctx)); > net->out_ctx = (core_output_filter_ctx_t *)ctx; > -rv = apr_socket_opt_set(net->client_socket, APR_SO_NONBLOCK, 1); > -if (rv != APR_SUCCESS) { > -return rv; > -} > /* > * Need to create tmp brigade with correct lifetime. Passing > * NULL to apr_brigade_split_ex would result in a brigade > > I'll run it through the test framework on Linux before committing. > -- Born in Roswell... married an alien... http://emptyhammock.com/
RE: core filters vs non-blocking socket (was Re: Fix for Windows bug#52476)
> -Original Message- > From: Jeff Trawick [mailto:] > Sent: Montag, 13. August 2012 15:35 > To: dev@httpd.apache.org > Subject: Re: core filters vs non-blocking socket (was Re: Fix for > Windows bug#52476) > > On Mon, Aug 13, 2012 at 9:31 AM, Plüm, Rüdiger, Vodafone Group > wrote: > > > > And if we do a read with a timeout don't we do a poll with a timeout > where it does not > > matter whether the socket is blocking or non blocking? > > from a high-level, yes; but there is some logic in APR to guess when > data is already available to read, and that requires that the socket > is non-blocking in case the guess is incorrect (that logic is > associated with the APR_INCOMPLETE_READ flag) Thanks for the detailed clarification. Regards Rüdiger
Re: core filters vs non-blocking socket (was Re: Fix for Windows bug#52476)
On Mon, Aug 13, 2012 at 9:31 AM, Plüm, Rüdiger, Vodafone Group wrote: > > >> -Original Message- >> From: Joe Orton [mailto:jor...@redhat.com] >> Sent: Montag, 13. August 2012 14:32 >> To: dev@httpd.apache.org >> Subject: core filters vs non-blocking socket (was Re: Fix for Windows >> bug#52476) >> >> On Fri, Aug 10, 2012 at 01:31:07PM -0400, Jeff Trawick wrote: >> > We picked up that apr_socket_opt_set() from the async-dev branch with >> > r327872, though the timeout calls in there were changed subsequently. >> > I wonder if that call is stray and it doesn't get along with the >> > timeout handling on Windows because of the SO_RCVTIMEO/SO_SENDTIMEO >> > use on Windows, in lieu of non-blocking socket + poll like on Unix. >> > >> > At the time it was added, the new code was >> > >> > apr_socket_timeout_set(client_socket, 0) >> > apr_socket_opt_set(client_socket, APR_SO_NONBLOCK, 1) >> > >> > (redundant unless there was some APR glitch at the time) >> >> H, is this right? >> >> For event, the listening socket, and hence the accepted socket, is >> always set to non-blocking in the MPM. >> >> For non-event on Unix, the listening socket, and hence the accepted >> socket, is set to non-blocking IFF there are multiple listeners. >> >> So that line is not redundant in the non-event, single listener >> configuration on Unix... no? > > Don't we switch to non-blocking in apr_socket_timeout_set if we set the > timeout to 0? yes apr_status_t apr_socket_timeout_set(apr_socket_t *sock, apr_interval_time_t t) { apr_status_t stat; /* If our new timeout is non-negative and our old timeout was * negative, then we need to ensure that we are non-blocking. * Conversely, if our new timeout is negative and we had * non-negative timeout, we must make sure our socket is blocking. * We want to avoid calling fcntl more than necessary on the * socket. */ if (t >= 0 && sock->timeout < 0) { if (apr_is_option_set(sock, APR_SO_NONBLOCK) != 1) { if ((stat = sononblock(sock->socketdes)) != APR_SUCCESS) { return stat; } apr_set_option(sock, APR_SO_NONBLOCK, 1); } } else if (t < 0 && sock->timeout >= 0) { if (apr_is_option_set(sock, APR_SO_NONBLOCK) != 0) { if ((stat = soblock(sock->socketdes)) != APR_SUCCESS) { return stat; } apr_set_option(sock, APR_SO_NONBLOCK, 0); } } ... > And if we do a read with a timeout don't we do a poll with a timeout where it > does not > matter whether the socket is blocking or non blocking? from a high-level, yes; but there is some logic in APR to guess when data is already available to read, and that requires that the socket is non-blocking in case the guess is incorrect (that logic is associated with the APR_INCOMPLETE_READ flag) > > Or did I get confused now completely? no > > Regards > > Rüdiger > -- Born in Roswell... married an alien... http://emptyhammock.com/
RE: core filters vs non-blocking socket (was Re: Fix for Windows bug#52476)
> -Original Message- > From: Joe Orton [mailto:jor...@redhat.com] > Sent: Montag, 13. August 2012 14:32 > To: dev@httpd.apache.org > Subject: core filters vs non-blocking socket (was Re: Fix for Windows > bug#52476) > > On Fri, Aug 10, 2012 at 01:31:07PM -0400, Jeff Trawick wrote: > > We picked up that apr_socket_opt_set() from the async-dev branch with > > r327872, though the timeout calls in there were changed subsequently. > > I wonder if that call is stray and it doesn't get along with the > > timeout handling on Windows because of the SO_RCVTIMEO/SO_SENDTIMEO > > use on Windows, in lieu of non-blocking socket + poll like on Unix. > > > > At the time it was added, the new code was > > > > apr_socket_timeout_set(client_socket, 0) > > apr_socket_opt_set(client_socket, APR_SO_NONBLOCK, 1) > > > > (redundant unless there was some APR glitch at the time) > > H, is this right? > > For event, the listening socket, and hence the accepted socket, is > always set to non-blocking in the MPM. > > For non-event on Unix, the listening socket, and hence the accepted > socket, is set to non-blocking IFF there are multiple listeners. > > So that line is not redundant in the non-event, single listener > configuration on Unix... no? Don't we switch to non-blocking in apr_socket_timeout_set if we set the timeout to 0? And if we do a read with a timeout don't we do a poll with a timeout where it does not matter whether the socket is blocking or non blocking? Or did I get confused now completely? Regards Rüdiger
Re: core filters vs non-blocking socket (was Re: Fix for Windows bug#52476)
On Mon, Aug 13, 2012 at 8:32 AM, Joe Orton wrote: > On Fri, Aug 10, 2012 at 01:31:07PM -0400, Jeff Trawick wrote: >> We picked up that apr_socket_opt_set() from the async-dev branch with >> r327872, though the timeout calls in there were changed subsequently. >> I wonder if that call is stray and it doesn't get along with the >> timeout handling on Windows because of the SO_RCVTIMEO/SO_SENDTIMEO >> use on Windows, in lieu of non-blocking socket + poll like on Unix. >> >> At the time it was added, the new code was >> >> apr_socket_timeout_set(client_socket, 0) >> apr_socket_opt_set(client_socket, APR_SO_NONBLOCK, 1) >> >> (redundant unless there was some APR glitch at the time) > > H, is this right? > > For event, the listening socket, and hence the accepted socket, is > always set to non-blocking in the MPM. > > For non-event on Unix, the listening socket, and hence the accepted > socket, is set to non-blocking IFF there are multiple listeners. But the underlying descriptor for the accepted gets set to non-blocking soon after accept() (see below). > > So that line is not redundant in the non-event, single listener > configuration on Unix... no? > > Regards, Joe Background: With the APR socket implementation on Unix, the underlying socket descriptor is placed in non-blocking mode when the application (httpd) sets a timeout >= 0. For a single listener worker configuration with mod_reqtimeout disabled and the default Timeout of 60 seconds, here's what happens w.r.t. blocking and timeouts on the connected sockets once core_create_conn() is entered. This is with the bug present (i.e., with the needless apr_socket_opt_set()). At the time core_create_conn() is entered, the apr_socket_t has timeout -1 and the only option set is APR_TCP_NODELAY). * core_pre_connection() sets timeout to 6000 (this makes the underlying socket non-blocking on Unix) * SSL needs to read but first calls flush and ap_core_output_filter() makes the APR socket non-blocking (bug according to me ;) ) * also called from SSL flush, writev_nonblocking() sets timeout to 0 (that also ensures that the underlying socket is non-blocking) * writev_nonblocking() restores the timeout (6000) * also called from SSL flush, writev_nonblocking() sets timeout to 0 and then restores the timeout (6000) * in some complicated flow from default_handler through SSL, writev_nonblocking sets timeout to 0 then restores the timeout * lingering_close() sets timeout to 200 So the underlying socket descriptor is still made non-blocking on Unix even before the bug is encountered, as part of the timeout implementation. And that call to mark the socket non-blocking with apr_socket_opt_set() is out of sync with the rest of the code that sets timeout to 0 or >0 as necessary. (And of course it is out of sync in a way that exposes the difference on Windows.) Does that explanation work for you? -- Born in Roswell... married an alien... http://emptyhammock.com/
Re: Fix for Windows bug#52476
Also here it is running now without issues till now here with AcceptFilter-none+SSL Steffen -Original Message- From: Jeff Trawick Sent: Friday, August 10, 2012 7:43 PM Newsgroups: gmane.comp.apache.devel To: dev@httpd.apache.org Subject: Re: Fix for Windows bug#52476 This patch is testing great so far with the AcceptFilter-none+SSL scenario on Windows. Index: server/core_filters.c === --- server/core_filters.c (revision 1371377) +++ server/core_filters.c (working copy) @@ -391,10 +391,6 @@ if (ctx == NULL) { ctx = apr_pcalloc(c->pool, sizeof(*ctx)); net->out_ctx = (core_output_filter_ctx_t *)ctx; -rv = apr_socket_opt_set(net->client_socket, APR_SO_NONBLOCK, 1); -if (rv != APR_SUCCESS) { -return rv; -} /* * Need to create tmp brigade with correct lifetime. Passing * NULL to apr_brigade_split_ex would result in a brigade I'll run it through the test framework on Linux before committing.
core filters vs non-blocking socket (was Re: Fix for Windows bug#52476)
On Fri, Aug 10, 2012 at 01:31:07PM -0400, Jeff Trawick wrote: > We picked up that apr_socket_opt_set() from the async-dev branch with > r327872, though the timeout calls in there were changed subsequently. > I wonder if that call is stray and it doesn't get along with the > timeout handling on Windows because of the SO_RCVTIMEO/SO_SENDTIMEO > use on Windows, in lieu of non-blocking socket + poll like on Unix. > > At the time it was added, the new code was > > apr_socket_timeout_set(client_socket, 0) > apr_socket_opt_set(client_socket, APR_SO_NONBLOCK, 1) > > (redundant unless there was some APR glitch at the time) H, is this right? For event, the listening socket, and hence the accepted socket, is always set to non-blocking in the MPM. For non-event on Unix, the listening socket, and hence the accepted socket, is set to non-blocking IFF there are multiple listeners. So that line is not redundant in the non-event, single listener configuration on Unix... no? Regards, Joe
RE: svn commit: r1372054 - in /httpd/httpd/trunk: CHANGES server/util.c
> -Original Message- > From: Nick Kew [mailto:n...@webthing.com] > Sent: Montag, 13. August 2012 13:11 > To: dev@httpd.apache.org > Subject: Re: svn commit: r1372054 - in /httpd/httpd/trunk: CHANGES > server/util.c > > > On 12 Aug 2012, at 14:15, Ruediger Pluem wrote: > > > ap_strcmp_match seems to be a lot of overhead for just prefix matching > a string. > > How about > > > > strncmp("application/x-www-form-urlencoded", ct, 33) > > Either way, shouldn't it be a case-insensitive match? Good point. Regards Rüdiger
Re: svn commit: r1372054 - in /httpd/httpd/trunk: CHANGES server/util.c
On 12 Aug 2012, at 14:15, Ruediger Pluem wrote: > ap_strcmp_match seems to be a lot of overhead for just prefix matching a > string. > How about > > strncmp("application/x-www-form-urlencoded", ct, 33) Either way, shouldn't it be a case-insensitive match? -- Nick Kew
Re: svn commit: r1372054 - in /httpd/httpd/trunk: CHANGES server/util.c
On 08/12/2012 03:15 PM, Ruediger Pluem wrote: > > > humbed...@apache.org wrote: >> Author: humbedooh >> Date: Sun Aug 12 07:45:55 2012 >> New Revision: 1372054 >> >> URL: http://svn.apache.org/viewvc?rev=1372054&view=rev >> Log: >> core: >> Be less strict when checking whether Content-Type is set to >> "application/x-www-form-urlencoded" >> when parsing POST data, or we risk losing data with an appended charset. >> >> PR 53698 >> Reported by: Petter Berntsen < sluggr gmail.com > >> >> Modified: >> httpd/httpd/trunk/CHANGES >> httpd/httpd/trunk/server/util.c >> > >> Modified: httpd/httpd/trunk/server/util.c >> URL: >> http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util.c?rev=1372054&r1=1372053&r2=1372054&view=diff >> == >> --- httpd/httpd/trunk/server/util.c (original) >> +++ httpd/httpd/trunk/server/util.c Sun Aug 12 07:45:55 2012 >> @@ -2406,7 +2406,7 @@ AP_DECLARE(int) ap_parse_form_data(reque >> >> /* sanity check - we only support forms for now */ >> ct = apr_table_get(r->headers_in, "Content-Type"); >> -if (!ct || strcmp("application/x-www-form-urlencoded", ct)) { >> +if (!ct || ap_strcmp_match(ct, "application/x-www-form-urlencoded*")) { > > ap_strcmp_match seems to be a lot of overhead for just prefix matching a > string. > How about > > strncmp("application/x-www-form-urlencoded", ct, 33) > > instead. > > Regards > > Rüdiger > Yeah, that approach seems better. Took me a couple of tries to commit the right version though - I blame the lack of coffee coupled with heavy partying last night. With regards Daniel.