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

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

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