Loading ...
Sorry, an error occurred while loading the content.

Re: [mp2] [patch] CGI::Cookie does not work prior to response phase

Expand Messages
  • Nick Tonkin
    ... D oh. ... what about eval require mod_perl ; which never gets checked? ... Eh? You said: if $r is not available, there are a few things to choose from:
    Message 1 of 16 , Apr 10, 2003
    • 0 Attachment
      On Fri, 11 Apr 2003, Stas Bekman wrote:

      > Nick Tonkin wrote:
      > [...]
      >
      > Your diff is the other way around, - should say what was removed and + what
      > was added, you just need to swap the order of files while doing diff-- the
      > newer file last...

      D'oh.

      >
      > > 1) is the use of Apache->request correct? For mp1 and mp2?
      >
      > Yes
      >
      > > 2) the code to check for mod_perl does not appear to do any error
      > > checking, even when evalling. (Again I note that I copied it from
      > > CGI.pm)
      >
      > It's correct, since it checks 'defined $mod_perl::VERSION'

      what about

      eval "require mod_perl";

      which never gets checked?

      >
      > [...]
      > > sub fetch {
      > > my $class = shift;
      > > - my $raw_cookie;
      > > - if ($MOD_PERL) {
      > > - my $r = shift || Apache->request();
      > > - if (defined $r) {
      > > - $raw_cookie = $r->headers_in->{'Cookie'};
      > > - } else {
      > > - if (defined %ENV) {
      >
      > that's wrong. sure you have always %ENV defined. I think it should be:
      >
      > if (!exists $ENV{QUERY_STRING})
      >
      > but read on:
      >
      > > - $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
      > > - } else {
      > > - die '%ENV has not been set. Turn SetupEnv on or ' .
      > > - 'run $r->subprocess_env()';
      > > - }
      > > - }
      > > - } else {
      > > - $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
      >
      > well, that's a tiny bit code dup. and also if you use Apache->request, you
      > don't need to use %ENV at all.

      Eh? You said:

      if $r is not available, there are a few things to choose from:
      1) can use Apache->request
      - Apache->request will die if not available
      2) can use %ENV
      - but SetupEnv might be off, so we can do:
      die "Turn SetupEnv on or run $r->subprocess_env"
      unless exists $ENV{QUERY_STRING};



      >
      > - if ($MOD_PERL) {
      > - my $r = shift || Apache->request();
      > - $raw_cookie = $r->headers_in->{'Cookie'};
      > - } else {
      > - $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
      > - }
      >
      > same change for 'sub raw_fetch'
      >
      > __________________________________________________________________
      > Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
      > http://stason.org/ mod_perl Guide ---> http://perl.apache.org
      > mailto:stas@... http://use.perl.org http://apacheweek.com
      > http://modperlbook.org http://apache.org http://ticketmaster.com
      >


      - nick

      --

      ~~~~~~~~~~~~~~~~~~~~
      Nick Tonkin {|8^)>
    • Nick Tonkin
      On Fri, 11 Apr 2003, Stas Bekman wrote:Nick Tonkin wrote: [...] Your diff is the other way around, - should say what was removed and + what was
      Message 2 of 16 , Apr 10, 2003
      • 0 Attachment
        On Fri, 11 Apr 2003, Stas Bekman wrote:

        > Nick Tonkin wrote:
        > [...]
        >
        > Your diff is the other way around, - should say what was removed and + what
        > was added, you just need to swap the order of files while doing diff-- the
        > newer file last...
        >
        > > 1) is the use of Apache->request correct? For mp1 and mp2?
        >
        > Yes
        >
        > > 2) the code to check for mod_perl does not appear to do any error
        > > checking, even when evalling. (Again I note that I copied it from
        > > CGI.pm)
        >
        > It's correct, since it checks 'defined $mod_perl::VERSION'
        >
        > [...]
        > > sub fetch {
        > > my $class = shift;
        > > - my $raw_cookie;
        > > - if ($MOD_PERL) {
        > > - my $r = shift || Apache->request();
        > > - if (defined $r) {
        > > - $raw_cookie = $r->headers_in->{'Cookie'};
        > > - } else {
        > > - if (defined %ENV) {
        >
        > that's wrong. sure you have always %ENV defined. I think it should be:
        >
        > if (!exists $ENV{QUERY_STRING})
        >
        > but read on:
        >
        > > - $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
        > > - } else {
        > > - die '%ENV has not been set. Turn SetupEnv on or ' .
        > > - 'run $r->subprocess_env()';
        > > - }
        > > - }
        > > - } else {
        > > - $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
        >
        > well, that's a tiny bit code dup. and also if you use Apache->request, you
        > don't need to use %ENV at all.
        >
        > - if ($MOD_PERL) {
        > - my $r = shift || Apache->request();
        > - $raw_cookie = $r->headers_in->{'Cookie'};
        > - } else {
        > - $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
        > - }
        >
        > same change for 'sub raw_fetch'
        >
        > __________________________________________________________________
        > Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
        > http://stason.org/ mod_perl Guide ---> http://perl.apache.org
        > mailto:stas@... http://use.perl.org http://apacheweek.com
        > http://modperlbook.org http://apache.org http://ticketmaster.com
        >

        New patch attached. It seemd more logical to test for $ENV{PATH_INFO} than
        $ENV{QUERY_STRING}, dunno if you agree.

        - nick

        --

        ~~~~~~~~~~~~~~~~~~~~
        Nick Tonkin {|8^)>
      • Stas Bekman
        ... it s checked by defined $mod_perl::VERSION ... that s correct. But since you do: $r || Apache- request the second option is irrelevant. Apache- request
        Message 3 of 16 , Apr 10, 2003
        • 0 Attachment
          Nick Tonkin wrote:

          >>>2) the code to check for mod_perl does not appear to do any error
          >>> checking, even when evalling. (Again I note that I copied it from
          >>> CGI.pm)
          >>
          >>It's correct, since it checks 'defined $mod_perl::VERSION'
          >
          >
          > what about
          >
          > eval "require mod_perl";
          >
          > which never gets checked?

          it's checked by 'defined $mod_perl::VERSION'

          >>well, that's a tiny bit code dup. and also if you use Apache->request, you
          >>don't need to use %ENV at all.
          >
          >
          > Eh? You said:
          >
          > if $r is not available, there are a few things to choose from:
          > 1) can use Apache->request
          > - Apache->request will die if not available
          > 2) can use %ENV
          > - but SetupEnv might be off, so we can do:
          > die "Turn SetupEnv on or run $r->subprocess_env"
          > unless exists $ENV{QUERY_STRING};

          that's correct. But since you do: $r || Apache->request the second option is
          irrelevant. Apache->request already throws an exception if it's not available.

          __________________________________________________________________
          Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
          http://stason.org/ mod_perl Guide ---> http://perl.apache.org
          mailto:stas@... http://use.perl.org http://apacheweek.com
          http://modperlbook.org http://apache.org http://ticketmaster.com
        • Stas Bekman
          Nick Tonkin wrote: The patch looks good (though I haven t tested it). One thing: [...] ... the only situation when CGI env vars might be unavailable is under
          Message 4 of 16 , Apr 10, 2003
          • 0 Attachment
            Nick Tonkin wrote:

            The patch looks good (though I haven't tested it). One thing:

            [...]
            > + my $r;
            > + $r = (shift || Apache->request()) if $MOD_PERL;
            > + if (defined $r) {
            > + $raw_cookie = $r->headers_in->{'Cookie'};
            > + } else {
            > + if (exists $ENV{PATH_INFO}) {
            > + $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
            > + } else {
            > + die '%ENV has not been set. Turn SetupEnv on or ' .
            > + 'run $r->subprocess_env()';
            > + }
            > + }


            the only situation when CGI env vars might be unavailable is under mp2, but
            since you already do:

            $r = (shift || Apache->request()) if $MOD_PERL;

            this extra die is not needed. You also don't need defined($r). All you need is:

            + my $r;
            + $r = (shift || Apache->request()) if $MOD_PERL;
            + if ($r) {
            + $raw_cookie = $r->headers_in->{'Cookie'};
            + } else {
            + $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
            + }

            however your logic could be made even more user-friendly if you did this:

            + my $r;
            + $r = shift if $MOD_PERL;
            + $r ||= eval { Apache->request() } if $MOD_PERL;
            + if ($r) {
            + $raw_cookie = $r->headers_in->{'Cookie'};
            + } else {
            + if (exists $ENV{PATH_INFO}) {
            + $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
            + } else {
            + die '%ENV has not been set. Turn SetupEnv on or ' .
            + 'run $r->subprocess_env()';
            + }
            + }

            Here, we use Apache->request only if it's available, and if $r was neither
            passed as an argument nor it's available through Apache->request, we try to
            use the third fallback solution and if even that not available, we die. Looks
            like the best solution to me.

            > It seemd more logical to test for $ENV{PATH_INFO} than
            > $ENV{QUERY_STRING}, dunno if you agree.

            I believe that they all are all always set no matter what, need to check the
            actual implementation. So I don't really care which one is used.

            __________________________________________________________________
            Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
            http://stason.org/ mod_perl Guide ---> http://perl.apache.org
            mailto:stas@... http://use.perl.org http://apacheweek.com
            http://modperlbook.org http://apache.org http://ticketmaster.com
          • Nick Tonkin
            On Fri, 11 Apr 2003, Stas Bekman wrote:Nick Tonkin wrote: The patch looks good (though I haven t tested it). One thing: [...] + my $r; +
            Message 5 of 16 , Apr 10, 2003
            • 0 Attachment
              On Fri, 11 Apr 2003, Stas Bekman wrote:

              > Nick Tonkin wrote:
              >
              > The patch looks good (though I haven't tested it). One thing:
              >
              > [...]
              > > + my $r;
              > > + $r = (shift || Apache->request()) if $MOD_PERL;
              > > + if (defined $r) {
              > > + $raw_cookie = $r->headers_in->{'Cookie'};
              > > + } else {
              > > + if (exists $ENV{PATH_INFO}) {
              > > + $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
              > > + } else {
              > > + die '%ENV has not been set. Turn SetupEnv on or ' .
              > > + 'run $r->subprocess_env()';
              > > + }
              > > + }
              >
              >
              > the only situation when CGI env vars might be unavailable is under mp2, but
              > since you already do:
              >
              > $r = (shift || Apache->request()) if $MOD_PERL;
              >
              > this extra die is not needed. You also don't need defined($r). All you need is:
              >
              > + my $r;
              > + $r = (shift || Apache->request()) if $MOD_PERL;
              > + if ($r) {
              > + $raw_cookie = $r->headers_in->{'Cookie'};
              > + } else {
              > + $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
              > + }
              >
              > however your logic could be made even more user-friendly if you did this:
              >
              > + my $r;
              > + $r = shift if $MOD_PERL;
              > + $r ||= eval { Apache->request() } if $MOD_PERL;
              > + if ($r) {
              > + $raw_cookie = $r->headers_in->{'Cookie'};
              > + } else {
              > + if (exists $ENV{PATH_INFO}) {
              > + $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
              > + } else {
              > + die '%ENV has not been set. Turn SetupEnv on or ' .
              > + 'run $r->subprocess_env()';
              > + }
              > + }
              >
              > Here, we use Apache->request only if it's available, and if $r was neither
              > passed as an argument nor it's available through Apache->request, we try to
              > use the third fallback solution and if even that not available, we die. Looks
              > like the best solution to me.

              Agreed.

              >
              > > It seemd more logical to test for $ENV{PATH_INFO} than
              > > $ENV{QUERY_STRING}, dunno if you agree.
              >
              > I believe that they all are all always set no matter what, need to check the
              > actual implementation. So I don't really care which one is used.

              Hm. So $ENV{QUERY_STRING} gets set (to what?) when there is no query string?
              Whatever. I don't care either, I just think it's more intuitive to assume that
              there's always going to be a PATH_INFO ...


              So, attached is (yet) another diff, and then someone should test it on mp1, and then
              you'll send it to Lincoln?

              - nick

              --

              ~~~~~~~~~~~~~~~~~~~~
              Nick Tonkin {|8^)>
            • Stas Bekman
              ... It can be QUERY_STRING, but not PATH_INFO, since the former is always set, whereas the latter is not: apache-1.3/src/main/util_script.c:381:
              Message 6 of 16 , Apr 10, 2003
              • 0 Attachment
                > > It seemd more logical to test for $ENV{PATH_INFO} than
                > > $ENV{QUERY_STRING}, dunno if you agree.
                >
                > I believe that they all are all always set no matter what, need to check
                > the actual implementation. So I don't really care which one is used.

                It can be QUERY_STRING, but not PATH_INFO, since the former is always set,
                whereas the latter is not:

                apache-1.3/src/main/util_script.c:381: ap_table_setn(e, "QUERY_STRING",
                r->args ? r->args : "");

                httpd-2.0/server/util_script.c:370: apr_table_setn(e, "QUERY_STRING",
                r->args ? r->args : "");

                We can choose from: SERVER_PROTOCOL, REQUEST_METHOD, QUERY_STRING and REQUEST_URI.

                __________________________________________________________________
                Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
                http://stason.org/ mod_perl Guide ---> http://perl.apache.org
                mailto:stas@... http://use.perl.org http://apacheweek.com
                http://modperlbook.org http://apache.org http://ticketmaster.com
              • Nick Tonkin
                On Fri, 11 Apr 2003, Stas Bekman wrote: It seemd more logical to test for $ENV{PATH_INFO} than $ENV{QUERY_STRING}, dunno if you agree.
                Message 7 of 16 , Apr 10, 2003
                • 0 Attachment
                  On Fri, 11 Apr 2003, Stas Bekman wrote:

                  >
                  > > > It seemd more logical to test for $ENV{PATH_INFO} than
                  > > > $ENV{QUERY_STRING}, dunno if you agree.
                  > >
                  > > I believe that they all are all always set no matter what, need to check
                  > > the actual implementation. So I don't really care which one is used.
                  >
                  > It can be QUERY_STRING, but not PATH_INFO, since the former is always set,
                  > whereas the latter is not:
                  >
                  > apache-1.3/src/main/util_script.c:381: ap_table_setn(e, "QUERY_STRING",
                  > r->args ? r->args : "");
                  >
                  > httpd-2.0/server/util_script.c:370: apr_table_setn(e, "QUERY_STRING",
                  > r->args ? r->args : "");
                  >
                  > We can choose from: SERVER_PROTOCOL, REQUEST_METHOD, QUERY_STRING and REQUEST_URI.
                  >
                  > __________________________________________________________________
                  > Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
                  > http://stason.org/ mod_perl Guide ---> http://perl.apache.org
                  > mailto:stas@... http://use.perl.org http://apacheweek.com
                  > http://modperlbook.org http://apache.org http://ticketmaster.com
                  >

                  Heh. Missed you by a sec. Here's a new patch.

                  - nick

                  --

                  ~~~~~~~~~~~~~~~~~~~~
                  Nick Tonkin {|8^)>
                • Nick Tonkin
                  ... But then does nothing if it s not defined. No graceful handling in other words. Or do I miss something? - nick -- ~~~~~~~~~~~~~~~~~~~~ Nick Tonkin {|8^)
                  Message 8 of 16 , Apr 10, 2003
                  • 0 Attachment
                    On Thu, 10 Apr 2003, Nick Tonkin wrote:

                    > On Fri, 11 Apr 2003, Stas Bekman wrote:
                    >
                    > > Nick Tonkin wrote:
                    > > [...]
                    > >
                    > > Your diff is the other way around, - should say what was removed and + what
                    > > was added, you just need to swap the order of files while doing diff-- the
                    > > newer file last...
                    > >
                    > > > 1) is the use of Apache->request correct? For mp1 and mp2?
                    > >
                    > > Yes
                    > >
                    > > > 2) the code to check for mod_perl does not appear to do any error
                    > > > checking, even when evalling. (Again I note that I copied it from
                    > > > CGI.pm)
                    > >
                    > > It's correct, since it checks 'defined $mod_perl::VERSION'

                    But then does nothing if it's not defined. No graceful handling in other words.
                    Or do I miss something?

                    - nick

                    --

                    ~~~~~~~~~~~~~~~~~~~~
                    Nick Tonkin {|8^)>
                  • Stas Bekman
                    ... Yup, I knew that this will happen. It misses this comment. I ve submitted a patch to Lincoln to add this comment. if (exists $ENV{MOD_PERL}) { eval
                    Message 9 of 16 , Apr 10, 2003
                    • 0 Attachment
                      Nick Tonkin wrote:
                      > On Thu, 10 Apr 2003, Nick Tonkin wrote:
                      >
                      >
                      >>On Fri, 11 Apr 2003, Stas Bekman wrote:
                      >>
                      >>
                      >>>Nick Tonkin wrote:
                      >>>[...]
                      >>>
                      >>>Your diff is the other way around, - should say what was removed and + what
                      >>>was added, you just need to swap the order of files while doing diff-- the
                      >>>newer file last...
                      >>>
                      >>>
                      >>>>1) is the use of Apache->request correct? For mp1 and mp2?
                      >>>
                      >>>Yes
                      >>>
                      >>>
                      >>>>2) the code to check for mod_perl does not appear to do any error
                      >>>> checking, even when evalling. (Again I note that I copied it from
                      >>>> CGI.pm)
                      >>>
                      >>>It's correct, since it checks 'defined $mod_perl::VERSION'
                      >>
                      >
                      > But then does nothing if it's not defined. No graceful handling in other words.
                      > Or do I miss something?

                      Yup, I knew that this will happen. It misses this comment. I've submitted a
                      patch to Lincoln to add this comment.

                      if (exists $ENV{MOD_PERL}) {
                      eval "require mod_perl";
                      # mod_perl handlers may run system() on scripts using CGI.pm
                      # - make sure so we don't get fooled by inherited $ENV{MOD_PERL}
                      if (defined $mod_perl::VERSION) {
                      if ($mod_perl::VERSION >= 1.99) {

                      it talks about system() calls made from mod_perl scripts. When this happens
                      $ENV{MOD_PERL} exists, but eval fails, $mod_perl::VERSION undefined == we
                      don't run under mod_perl.

                      __________________________________________________________________
                      Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
                      http://stason.org/ mod_perl Guide ---> http://perl.apache.org
                      mailto:stas@... http://use.perl.org http://apacheweek.com
                      http://modperlbook.org http://apache.org http://ticketmaster.com
                    • Stas Bekman
                      ... Functionalty seems to be alright, I d just do one last tiny change: + my $raw_cookie; + my $r = shift; + $r ||= eval { Apache- request() } if
                      Message 10 of 16 , Apr 10, 2003
                      • 0 Attachment
                        > +# Fetch a list of cookies from the environment or the incoming headers and
                        > +# return as a hash. The cookies are parsed as normal escaped URL data.
                        > sub fetch {
                        > my $class = shift;
                        > - my $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
                        > + my $raw_cookie;
                        > + my $r;
                        > + $r = shift if $MOD_PERL;
                        > + $r ||= eval { Apache->request() } if $MOD_PERL;
                        > + if ($r) {
                        > + $raw_cookie = $r->headers_in->{'Cookie'};
                        > + } else {
                        > + if (exists $ENV{REQUEST_URI}) {
                        > + $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
                        > + } else {
                        > + die '%ENV has not been set. Turn SetupEnv on or ' .
                        > + 'run $r->subprocess_env()';
                        > + }
                        > + }
                        > return () unless $raw_cookie;
                        > return $class->parse($raw_cookie);
                        > }

                        Functionalty seems to be alright, I'd just do one last tiny change:

                        + my $raw_cookie;
                        + my $r = shift;
                        + $r ||= eval { Apache->request() } if $MOD_PERL;
                        + if ($r) {
                        + $raw_cookie = $r->headers_in->{'Cookie'};
                        + } else {
                        + if (exists $ENV{REQUEST_URI}) {
                        + $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
                        + } else {
                        + die '%ENV has not been set. Turn SetupEnv on or ' .
                        + 'run $r->subprocess_env()';
                        + }
                        + }

                        but yours is just fine (though if you decide to go with yours pack the two if
                        $MOD_PERL together).

                        __________________________________________________________________
                        Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
                        http://stason.org/ mod_perl Guide ---> http://perl.apache.org
                        mailto:stas@... http://use.perl.org http://apacheweek.com
                        http://modperlbook.org http://apache.org http://ticketmaster.com
                      • Nick Tonkin
                        On Fri, 11 Apr 2003, Stas Bekman wrote: +# Fetch a list of cookies from the environment or the incoming headers and +# return as a hash. The cookies
                        Message 11 of 16 , Apr 10, 2003
                        • 0 Attachment
                          On Fri, 11 Apr 2003, Stas Bekman wrote:

                          >
                          > > +# Fetch a list of cookies from the environment or the incoming headers and
                          > > +# return as a hash. The cookies are parsed as normal escaped URL data.
                          > > sub fetch {
                          > > my $class = shift;
                          > > - my $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
                          > > + my $raw_cookie;
                          > > + my $r;
                          > > + $r = shift if $MOD_PERL;
                          > > + $r ||= eval { Apache->request() } if $MOD_PERL;
                          > > + if ($r) {
                          > > + $raw_cookie = $r->headers_in->{'Cookie'};
                          > > + } else {
                          > > + if (exists $ENV{REQUEST_URI}) {
                          > > + $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
                          > > + } else {
                          > > + die '%ENV has not been set. Turn SetupEnv on or ' .
                          > > + 'run $r->subprocess_env()';
                          > > + }
                          > > + }
                          > > return () unless $raw_cookie;
                          > > return $class->parse($raw_cookie);
                          > > }
                          >
                          > Functionalty seems to be alright, I'd just do one last tiny change:

                          Done. Patch attached.

                          >
                          > + my $raw_cookie;
                          > + my $r = shift;
                          > + $r ||= eval { Apache->request() } if $MOD_PERL;
                          > + if ($r) {
                          > + $raw_cookie = $r->headers_in->{'Cookie'};
                          > + } else {
                          > + if (exists $ENV{REQUEST_URI}) {
                          > + $raw_cookie = $ENV{HTTP_COOKIE} || $ENV{COOKIE};
                          > + } else {
                          > + die '%ENV has not been set. Turn SetupEnv on or ' .
                          > + 'run $r->subprocess_env()';
                          > + }
                          > + }
                          >
                          > but yours is just fine (though if you decide to go with yours pack the two if
                          > $MOD_PERL together).
                          >
                          > __________________________________________________________________
                          > Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
                          > http://stason.org/ mod_perl Guide ---> http://perl.apache.org
                          > mailto:stas@... http://use.perl.org http://apacheweek.com
                          > http://modperlbook.org http://apache.org http://ticketmaster.com
                          >


                          - nick

                          --

                          ~~~~~~~~~~~~~~~~~~~~
                          Nick Tonkin {|8^)>
                        • Stas Bekman
                          ... it s set to . Notice that we check: exists $ENV{QUERY_STRING} may be using REQUEST_METHOD is better. ... which is a wrong assumption I suppose, otherwise
                          Message 12 of 16 , Apr 10, 2003
                          • 0 Attachment
                            > Hm. So $ENV{QUERY_STRING} gets set (to what?) when there is no query string?

                            it's set to "". Notice that we check:

                            exists $ENV{QUERY_STRING}

                            may be using REQUEST_METHOD is better.

                            > Whatever. I don't care either, I just think it's more intuitive to assume that
                            > there's always going to be a PATH_INFO ...

                            which is a wrong assumption I suppose, otherwise it's would be always set in
                            C. You are probably confusing it with REQUEST_URI, which is always there and
                            also an intuitive choice.

                            __________________________________________________________________
                            Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
                            http://stason.org/ mod_perl Guide ---> http://perl.apache.org
                            mailto:stas@... http://use.perl.org http://apacheweek.com
                            http://modperlbook.org http://apache.org http://ticketmaster.com
                          • Stas Bekman
                            ... Great! mp1 has a test which uses CGI::Cookie, so you probably want to test that it works. Also porting the test to mp2 s test suite would be great too.
                            Message 13 of 16 , Apr 10, 2003
                            • 0 Attachment
                              > Done. Patch attached.

                              Great!

                              mp1 has a test which uses CGI::Cookie, so you probably want to test that it
                              works. Also porting the test to mp2's test suite would be great too.

                              __________________________________________________________________
                              Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
                              http://stason.org/ mod_perl Guide ---> http://perl.apache.org
                              mailto:stas@... http://use.perl.org http://apacheweek.com
                              http://modperlbook.org http://apache.org http://ticketmaster.com
                            • Nick Tonkin
                              ... As you can see that s what I went with. - nick -- ~~~~~~~~~~~~~~~~~~~~ Nick Tonkin {|8^)
                              Message 14 of 16 , Apr 10, 2003
                              • 0 Attachment
                                On Fri, 11 Apr 2003, Stas Bekman wrote:

                                >
                                > > Hm. So $ENV{QUERY_STRING} gets set (to what?) when there is no query string?
                                >
                                > it's set to "". Notice that we check:
                                >
                                > exists $ENV{QUERY_STRING}
                                >
                                > may be using REQUEST_METHOD is better.
                                >
                                > > Whatever. I don't care either, I just think it's more intuitive to assume that
                                > > there's always going to be a PATH_INFO ...
                                >
                                > which is a wrong assumption I suppose, otherwise it's would be always set in
                                > C. You are probably confusing it with REQUEST_URI, which is always there and
                                > also an intuitive choice.
                                >

                                As you can see that's what I went with.

                                - nick

                                --

                                ~~~~~~~~~~~~~~~~~~~~
                                Nick Tonkin {|8^)>
                              • Nick Tonkin
                                ... I don t have mp1 installed here ... and I m sure done for the night. Not sure when I can get any more mod_perl time ... - nick -- ~~~~~~~~~~~~~~~~~~~~ Nick
                                Message 15 of 16 , Apr 10, 2003
                                • 0 Attachment
                                  On Fri, 11 Apr 2003, Stas Bekman wrote:

                                  >
                                  > > Done. Patch attached.
                                  >
                                  > Great!
                                  >
                                  > mp1 has a test which uses CGI::Cookie, so you probably want to test that it
                                  > works. Also porting the test to mp2's test suite would be great too.

                                  I don't have mp1 installed here ... and I'm sure done for the night. Not sure
                                  when I can get any more mod_perl time ...

                                  - nick

                                  --

                                  ~~~~~~~~~~~~~~~~~~~~
                                  Nick Tonkin {|8^)>
                                • Stas Bekman
                                  ... I tested your patch against the mp1 test suite, so we now have Apache- request path tested.
                                  Message 16 of 16 , Apr 10, 2003
                                  • 0 Attachment
                                    Nick Tonkin wrote:
                                    > On Fri, 11 Apr 2003, Stas Bekman wrote:
                                    >
                                    >
                                    >>>Done. Patch attached.
                                    >>
                                    >>Great!
                                    >>
                                    >>mp1 has a test which uses CGI::Cookie, so you probably want to test that it
                                    >>works. Also porting the test to mp2's test suite would be great too.
                                    >
                                    >
                                    > I don't have mp1 installed here ... and I'm sure done for the night. Not sure
                                    > when I can get any more mod_perl time ...

                                    I tested your patch against the mp1 test suite, so we now have Apache->request
                                    path tested.

                                    __________________________________________________________________
                                    Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
                                    http://stason.org/ mod_perl Guide ---> http://perl.apache.org
                                    mailto:stas@... http://use.perl.org http://apacheweek.com
                                    http://modperlbook.org http://apache.org http://ticketmaster.com
                                  Your message has been successfully submitted and would be delivered to recipients shortly.