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
    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 1 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 2 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 3 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 4 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 5 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 6 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.