Final Reminder: Community Over Code call for presentations closing soon
[Note: You're receiving this email because you are subscribed to one or more project dev@ mailing lists at the Apache Software Foundation.] This is your final reminder that the Call for Presentations for Community Over Code (formerly known as ApacheCon) is closing soon - on Thursday, 13 July 2023 at 23:59:59 GMT. https://communityovercode.org/call-for-presentations/ We are looking for talk proposals on all topics related to ASF projects and open source software. The event will be held in Halifax, Nova Scotia, Octiber 7th through 10th. More details about the event may be found on the event website at https://communityovercode.org/ Rich, for the event planners
Re: svn commit: r1910662 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c
On 6/28/23 3:55 PM, Eric Covener wrote:
> On Wed, Jun 28, 2023 at 9:08 AM Ruediger Pluem wrote:
>>
>>
>>
>> On 6/28/23 2:37 PM, cove...@apache.org wrote:
>>> Author: covener
>>> Date: Wed Jun 28 12:37:50 2023
>>> New Revision: 1910662
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1910662&view=rev
>>> Log:
>>> back to original patch in PR66672
>>>
>>> Since QSNONE will skip splitoutqueryargs, it's where we should fixup the
>>> trailing ?
>>>
>>> Modified:
>>> httpd/httpd/trunk/modules/mappers/mod_rewrite.c
>>>
>>> Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
>>> URL:
>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1910662&r1=1910661&r2=1910662&view=diff
>>> ==
>>> --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
>>> +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 28 12:37:50 2023
>>> @@ -3909,10 +3909,12 @@ static const char *cmd_rewriterule(cmd_p
>>> }
>>>
>>> if (*(a2_end-1) == '?') {
>>> -*(a2_end-1) = '\0'; /* trailing ? has done its job */
>>> /* a literal ? at the end of the unsubstituted rewrite rule */
>>> -if (!(newrule->flags & RULEFLAG_QSAPPEND))
>>> -{
>>> +if (!(newrule->flags & RULEFLAG_QSAPPEND)) {
>>> +/* trailing ? has done its job. with QSA, splitoutqueryargs
>>> + * will handle it
>>
>> I think only if QSLAST is set. Otherwise substitutions or the matching URL
>> may place a '?' left of the terminating '?' that came
>> in as encoded '?' and thus would add the original querystring as "&&" +
>> r->args. Hence we would end up with
>>
>> > substitution>+ "?&&" + r->args.
>>
>> Hence I think it should be:
>>
>>if (!(newrule->flags & RULEFLAG_QSAPPEND)) {
>>/* trailing ? has done its job.
>> */
>>*(a2_end-1) = '\0';
>>newrule->flags |= RULEFLAG_QSNONE;
>>}
>>else {
>>/* with QSA, splitoutqueryargs will handle it if RULEFLAG_QSLAST
>> is set.
>>newrule->flags |= RULEFLAG_QSLAST;
>>}
>>
>
> flipping it around and refreshing some comments:
>
> if (*(a2_end-1) == '?') {
> /* a literal ? at the end of the unsubstituted rewrite rule */
> if (newrule->flags & RULEFLAG_QSAPPEND) {
>/* with QSA, splitoutqueryargs will safely handle it if
> RULEFLAG_QSLAST is set */
>newrule->flags |= RULEFLAG_QSLAST;
> }
> else {
> /* avoid getting a a query string via inadvertent capture */
> newrule->flags |= RULEFLAG_QSNONE;
> /* trailing ? has done its job, but splitoutqueryargs will
> not chop it off */
> *(a2_end-1) = '\0';
>}
> }
> else if (newrule->flags & RULEFLAG_QSDISCARD) {
> if (NULL == ap_strchr(newrule->output, '?')) {
> newrule->flags |= RULEFLAG_QSNONE;
> }
> }
>
> close?
>
Looks fine.
Regards
Rüdiger
Re: svn commit: r1910662 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c
On Wed, Jun 28, 2023 at 9:08 AM Ruediger Pluem wrote:
>
>
>
> On 6/28/23 2:37 PM, cove...@apache.org wrote:
> > Author: covener
> > Date: Wed Jun 28 12:37:50 2023
> > New Revision: 1910662
> >
> > URL: http://svn.apache.org/viewvc?rev=1910662&view=rev
> > Log:
> > back to original patch in PR66672
> >
> > Since QSNONE will skip splitoutqueryargs, it's where we should fixup the
> > trailing ?
> >
> > Modified:
> > httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> >
> > Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> > URL:
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1910662&r1=1910661&r2=1910662&view=diff
> > ==
> > --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> > +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 28 12:37:50 2023
> > @@ -3909,10 +3909,12 @@ static const char *cmd_rewriterule(cmd_p
> > }
> >
> > if (*(a2_end-1) == '?') {
> > -*(a2_end-1) = '\0'; /* trailing ? has done its job */
> > /* a literal ? at the end of the unsubstituted rewrite rule */
> > -if (!(newrule->flags & RULEFLAG_QSAPPEND))
> > -{
> > +if (!(newrule->flags & RULEFLAG_QSAPPEND)) {
> > +/* trailing ? has done its job. with QSA, splitoutqueryargs
> > + * will handle it
>
> I think only if QSLAST is set. Otherwise substitutions or the matching URL
> may place a '?' left of the terminating '?' that came
> in as encoded '?' and thus would add the original querystring as "&&" +
> r->args. Hence we would end up with
>
> substitution>+ "?&&" + r->args.
>
> Hence I think it should be:
>
>if (!(newrule->flags & RULEFLAG_QSAPPEND)) {
>/* trailing ? has done its job.
> */
>*(a2_end-1) = '\0';
>newrule->flags |= RULEFLAG_QSNONE;
>}
>else {
>/* with QSA, splitoutqueryargs will handle it if RULEFLAG_QSLAST
> is set.
>newrule->flags |= RULEFLAG_QSLAST;
>}
>
flipping it around and refreshing some comments:
if (*(a2_end-1) == '?') {
/* a literal ? at the end of the unsubstituted rewrite rule */
if (newrule->flags & RULEFLAG_QSAPPEND) {
/* with QSA, splitoutqueryargs will safely handle it if
RULEFLAG_QSLAST is set */
newrule->flags |= RULEFLAG_QSLAST;
}
else {
/* avoid getting a a query string via inadvertent capture */
newrule->flags |= RULEFLAG_QSNONE;
/* trailing ? has done its job, but splitoutqueryargs will
not chop it off */
*(a2_end-1) = '\0';
}
}
else if (newrule->flags & RULEFLAG_QSDISCARD) {
if (NULL == ap_strchr(newrule->output, '?')) {
newrule->flags |= RULEFLAG_QSNONE;
}
}
close?
--
Eric Covener
cove...@gmail.com
Re: svn commit: r1910662 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c
On 6/28/23 2:37 PM, cove...@apache.org wrote:
> Author: covener
> Date: Wed Jun 28 12:37:50 2023
> New Revision: 1910662
>
> URL: http://svn.apache.org/viewvc?rev=1910662&view=rev
> Log:
> back to original patch in PR66672
>
> Since QSNONE will skip splitoutqueryargs, it's where we should fixup the
> trailing ?
>
> Modified:
> httpd/httpd/trunk/modules/mappers/mod_rewrite.c
>
> Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1910662&r1=1910661&r2=1910662&view=diff
> ==
> --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 28 12:37:50 2023
> @@ -3909,10 +3909,12 @@ static const char *cmd_rewriterule(cmd_p
> }
>
> if (*(a2_end-1) == '?') {
> -*(a2_end-1) = '\0'; /* trailing ? has done its job */
> /* a literal ? at the end of the unsubstituted rewrite rule */
> -if (!(newrule->flags & RULEFLAG_QSAPPEND))
> -{
> +if (!(newrule->flags & RULEFLAG_QSAPPEND)) {
> +/* trailing ? has done its job. with QSA, splitoutqueryargs
> + * will handle it
I think only if QSLAST is set. Otherwise substitutions or the matching URL may
place a '?' left of the terminating '?' that came
in as encoded '?' and thus would add the original querystring as "&&" +
r->args. Hence we would end up with
+ "?&&" + r->args.
Hence I think it should be:
if (!(newrule->flags & RULEFLAG_QSAPPEND)) {
/* trailing ? has done its job.
*/
*(a2_end-1) = '\0';
newrule->flags |= RULEFLAG_QSNONE;
}
else {
/* with QSA, splitoutqueryargs will handle it if RULEFLAG_QSLAST is
set.
newrule->flags |= RULEFLAG_QSLAST;
}
Regards
Rüdiger
Re: svn commit: r1910650 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c
On Wed, Jun 28, 2023 at 8:14 AM Ruediger Pluem wrote:
>
>
>
> On 6/28/23 12:38 PM, cove...@apache.org wrote:
> > Author: covener
> > Date: Wed Jun 28 10:38:27 2023
> > New Revision: 1910650
> >
> > URL: http://svn.apache.org/viewvc?rev=1910650&view=rev
> > Log:
> > act more like pre-r1908097 with rewrite
> >
> > - be more conservative with setting QSNONE
> > - allow the query to be split as historically, but then drop r->args if
> > QSNONE
> >
> >
> > Modified:
> > httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> >
> > Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> > URL:
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1910650&r1=1910649&r2=1910650&view=diff
> > ==
> > --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> > +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 28 10:38:27 2023
> > @@ -806,12 +806,6 @@ static void splitout_queryargs(request_r
> > int qsdiscard = flags & RULEFLAG_QSDISCARD;
> > int qslast = flags & RULEFLAG_QSLAST;
> >
> > -if (flags & RULEFLAG_QSNONE) {
> > -rewritelog(r, 2, NULL, "discarding query string, no parse from
> > substitution");
> > -r->args = NULL;
> > -return;
> > -}
> > -
> > /* don't touch, unless it's a scheme for which a query string makes
> > sense.
> > * See RFC 1738 and RFC 2368.
> > */
> > @@ -853,6 +847,10 @@ static void splitout_queryargs(request_r
> > r->args[len-1] = '\0';
> > }
> > }
> > +if (flags & RULEFLAG_QSNONE) {
> > +rewritelog(r, 2, NULL, "discarding query string, no parse from
> > substitution");
> > +r->args = NULL;
> > +}
>
> Why moving it here? This means that we remove '?' that where originally
> encoded in the URL and everything after this from the
> resulting URL due to the
>
> *q++ = '\0';
>
> above, but we don't put them in r->args either if RULEFLAG_QSNONE is set e.g.
> if we end on a '?' in the rewrite template and no
> QSA / QSD being set. I think in this case we should not touch any '?' in the
> result and just treat them as decoded '?' that get
> renencoded e.g. when forwarding the URL to a backend.
>
I see what you mean. I was overly focused on getting closer to the
original behavior and just ensuring an unexpected query doesn't sneak
in, but this will truncate the path if there is a capture/substitution
of an encoded query.
Re: svn commit: r1910650 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c
On 6/28/23 12:38 PM, cove...@apache.org wrote:
> Author: covener
> Date: Wed Jun 28 10:38:27 2023
> New Revision: 1910650
>
> URL: http://svn.apache.org/viewvc?rev=1910650&view=rev
> Log:
> act more like pre-r1908097 with rewrite
>
> - be more conservative with setting QSNONE
> - allow the query to be split as historically, but then drop r->args if QSNONE
>
>
> Modified:
> httpd/httpd/trunk/modules/mappers/mod_rewrite.c
>
> Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1910650&r1=1910649&r2=1910650&view=diff
> ==
> --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 28 10:38:27 2023
> @@ -806,12 +806,6 @@ static void splitout_queryargs(request_r
> int qsdiscard = flags & RULEFLAG_QSDISCARD;
> int qslast = flags & RULEFLAG_QSLAST;
>
> -if (flags & RULEFLAG_QSNONE) {
> -rewritelog(r, 2, NULL, "discarding query string, no parse from
> substitution");
> -r->args = NULL;
> -return;
> -}
> -
> /* don't touch, unless it's a scheme for which a query string makes
> sense.
> * See RFC 1738 and RFC 2368.
> */
> @@ -853,6 +847,10 @@ static void splitout_queryargs(request_r
> r->args[len-1] = '\0';
> }
> }
> +if (flags & RULEFLAG_QSNONE) {
> +rewritelog(r, 2, NULL, "discarding query string, no parse from
> substitution");
> +r->args = NULL;
> +}
Why moving it here? This means that we remove '?' that where originally encoded
in the URL and everything after this from the
resulting URL due to the
*q++ = '\0';
above, but we don't put them in r->args either if RULEFLAG_QSNONE is set e.g.
if we end on a '?' in the rewrite template and no
QSA / QSD being set. I think in this case we should not touch any '?' in the
result and just treat them as decoded '?' that get
renencoded e.g. when forwarding the URL to a backend.
Regards
Rüdiger
Re: svn commit: r1910649 - in /httpd/httpd/trunk/test: clients/h2ws.c modules/http2/test_800_websockets.py modules/http2/ws_server.py
Hi Yann, > Am 28.06.2023 um 11:34 schrieb Yann Ylavic : > > Hi Stefan, > >> >> Modified: >>httpd/httpd/trunk/test/clients/h2ws.c >>httpd/httpd/trunk/test/modules/http2/test_800_websockets.py >>httpd/httpd/trunk/test/modules/http2/ws_server.py > > Are the ws tests supposed to run/pass in ci? They don't currently, and > ISTR from the PR that some python libs versions prevent them from > passing so I'm wondering if disabing isn't working there or if it's a > real failure.. Ah, forgot to merge those checks from the PR into trunk. Thanks for the reminder. Added in r1910654. Let's see if CI will be happy. > > > Regards; > Yann.
Re: svn commit: r1910649 - in /httpd/httpd/trunk/test: clients/h2ws.c modules/http2/test_800_websockets.py modules/http2/ws_server.py
Hi Stefan, > > Modified: > httpd/httpd/trunk/test/clients/h2ws.c > httpd/httpd/trunk/test/modules/http2/test_800_websockets.py > httpd/httpd/trunk/test/modules/http2/ws_server.py Are the ws tests supposed to run/pass in ci? They don't currently, and ISTR from the PR that some python libs versions prevent them from passing so I'm wondering if disabing isn't working there or if it's a real failure.. Regards; Yann.
