Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CGI::Fast overwrite CGI.pm pragmas if 'use'd after it #12

Closed
ache opened this issue Mar 8, 2015 · 5 comments
Closed

CGI::Fast overwrite CGI.pm pragmas if 'use'd after it #12

ache opened this issue Mar 8, 2015 · 5 comments
Labels
Documentation Unimportant

Comments

@ache
Copy link

ache commented Mar 8, 2015

Hi.
From the earlier thread you post as valid example:

use CGI qw/ :standard -no_xhtml -nosticky /;
use CGI::Fast;

This code drops at least -no_xhtml pragma (I don't check other pragmas, but it is possible that all of them are dropped), while the code with changed 'use' order

use CGI::Fast;
use CGI qw/ :standard -no_xhtml -nosticky /;

works normally. The rest of test code to make -no_xhtml loss visible is:

new CGI::Fast;
print header(), start_html();
@leejo
Copy link
Owner

leejo commented Mar 8, 2015

I've bisected this back to 53651f2 which seems to suggest it's not necessarily the import behaviour but rather the call to CGI->_reset_globals that's maybe not doing the right thing. I'll see if i can fix this although i'm not sure it's that important given the behaviour is approaching 10years in age (The fix might just be documentation).

leejo added a commit that referenced this issue Mar 8, 2015
@leejo
ref #12 - failing test case
dcdec7d 
load order of CGI and CGI::Fast can cause import pragmas in CGI to
be overwritten, which is probably not good behaviour. bisect shows
this was introduced in 53651f2. the fix maybe just do document this
behaviour given its age

squash this commit with the fix (or remove the test if the fix is
just documentation)
@ache
Copy link
Author

ache commented Mar 8, 2015

Given behavior is not equal to current one, so we can't treat it as 10 years old. Old one was to make CGI::Fast as drop-in CGI replacement, not as anything additional. In old code pragmas overwrite can't happens because this single line works there, so 'use CGI ...' was unneeded:

use CGI::Fast qw/ :standard -no_xhtml -nosticky /;

New way of separation of CGI::Fast as additional thing may be good but leads to unpleasant surprises, like this one or already discussed #11

To confirm my theory of old drop-in replacement way, this apparently incorrect code still works right now, so CGI::Fast reinitializes CGI with CGI::Fast own pragmas:

use CGI qw/ :standard /;
use CGI::Fast qw / -no_xhtml -nosticky /;

new CGI::Fast;
print header(), start_html();

@leejo
Copy link
Owner

leejo commented Mar 8, 2015

OK, i bisected #11 back to 9537f90 so clearly having CGI::Fast import call CGI import explicitly is buggy.

leejo added a commit that referenced this issue Mar 8, 2015
@leejo
Squashed commit of the following:
8dd3c99 
commit 73226ad8b5d36749f197581754be862506810325
Author: Lee Johnson <lee@givengain.ch>
Date:   Sun Mar 8 16:59:09 2015 +0100

    ref #11, #12 - perldoc and testing tweaks

    make it clear that CGI::Fast can be a drop in replacement for CGI
    but if using it with an explicit call to use CGI then the call to
    use CGI must happen after the call to use CGI::Fast to prevent any
    CGI import pragmas being overwritten

commit dcdec7d
Author: Lee Johnson <lee@givengain.ch>
Date:   Sun Mar 8 10:05:18 2015 +0100

    ref #12 - failing test case

    load order of CGI and CGI::Fast can cause import pragmas in CGI to
    be overwritten, which is probably not good behaviour. bisect shows
    this was introduced in 53651f2. the fix maybe just do document this
    behaviour given its age

    squash this commit with the fix (or remove the test if the fix is
    just documentation)
@leejo
Copy link
Owner

leejo commented Mar 8, 2015

Right, i've restored the behaviour of CGI::Fast to import functions of CGI into the caller's namespace in v4.13_04 of CGI.pm (which i have just uploaded to CPAN) - note the is a DEV release due to the ongoing testing of leejo/CGI.pm#162 . I've also uploaded an updated version of CGI::Fast with clarifying POD and a couple of extra regression tests for #11 and #12.

@leejo leejo closed this as completed Mar 8, 2015
@leejo
Copy link
Owner

leejo commented Mar 8, 2015

FYI leejo/CGI.pm@5f47d57 is the fix in CGI.pm

leejo added a commit that referenced this issue Mar 8, 2015
@leejo
ref #12 - fix regexp on CGI::VERSION
a44da92 
replace crappy regexp with an equally crappy regexp (that works on
this occasion)
jsonn pushed a commit to jsonn/pkgsrc that referenced this issue Apr 3, 2015
Update to 4.14:
9297a2b 
4.14 2015-04-01

    [ RELEASE NOTES ]
    - This release removes the AUTOLOAD and compile optimisations from CGI.pm
      that were introduced into CGI.pm twenty (20) years ago as a response to
      its large size, which meant there was a significant compile time penalty.

    - This optimisation is no longer relevant and makes the code difficult to
      deal with as well as making test coverage metrics incorrect. Benchmarks
      show that advantages of AUTOLOAD / lazy loading / deferred compile are
      less than 0.05s, which will be dwarfed by just about any meaningful code
      in a cgi script. If this is an issue for you then you should look at
      running CGI.pm in a persistent environment (FCGI, etc)

    - To offset some of the time added by removing the AUTOLOAD functionality
      the dependencies have been made runtime rather than compile time. The
      POD has also been split into its own file. CGI.pm now contains around
      4000 lines of code, which compared to some modules on CPAN isn't really
      that much

    - This essentially deprecates the -compile pragma and ->compile method. The
      -compile pragma will no longer do anything, whereas the ->compile method
      will raise a deprecation warning. More importantly this also REMOVES the
      -any pragma because as per the documentation this pragma needed to be
      "used with care or not at all" and allowing arbitrary HTML tags is almost
      certainly a bad idea. If you are using the -any pragma and using arbitrary
      tags (or have typo's in your code) your code will *BREAK*

    - Although this release should be back compatible (with the exception of any
      code using the -any pragma) you are encouraged to test it throughly as if
      you are doing anything out of the ordinary with CGI.pm (i.e. have bugs
      that may have been masked by the AUTOLOAD feature) you may see some issues.

    - References: GH #162, GH #137, GH #164

    [ FEATURES ]
    - CGI::Carp now has $CGI::Carp::FULL_PATH for displaying the full path to the
      offending script in error messages

    - CGI now has env_query_string() for getting the value of QUERY_STRING from the
      environment and not that fiddled with by CGI.pm (which is what query_string()
      does) (GH #161)

    - CGI::ENCODE_ENTITIES var added to control which chracters are encoded by the
      call to the HTML::Entities module - defaults to &<>"\x8b\x9b' (GH #157)

    [ SPEC / BUG FIXES ]
    - Add the multi_param method to :cgi export (thanks to xblitz for the patch
      and tests. GH #167)

    - Fix warning for lack of HTTP_USER_AGENT in CGI::Carp (GH #168)

    - Fix imports when called from CGI::Fast, restores the import of CGI functions
      into the callers namespace for users of CGI::Fast (GH leejo/cgi-fast#11 and
      GH leejo/cgi-fast#12)

    [ INTERNALS ]
    - Remove dependency on constant - internal DEBUG, XHTML_DTD and EBCDIC
      constants changes to $_DEBUG, $_XHTML_DTD, and $_EBCDIC

    [ DOCUMENTATION ]
    - Add missing documentation for env variable fetching routines (GH #163)
jsonn pushed a commit to jsonn/pkgsrc that referenced this issue Apr 22, 2015
Update to 4.15. Read the changelog below!
31f2136 
4.15 2015-04-20

    [ RELEASE NOTES ]
    - This release removes the AUTOLOAD and compile optimisations from CGI.pm
      that were introduced into CGI.pm twenty (20) years ago as a response to
      its large size, which meant there was a significant compile time penalty.

    - This optimisation is no longer relevant and makes the code difficult to
      deal with as well as making test coverage metrics incorrect. Benchmarks
      show that advantages of AUTOLOAD / lazy loading / deferred compile are
      less than 0.05s, which will be dwarfed by just about any meaningful code
      in a cgi script. If this is an issue for you then you should look at
      running CGI.pm in a persistent environment (FCGI, etc)

    - To offset some of the time added by removing the AUTOLOAD functionality
      the dependencies have been made runtime rather than compile time. The
      POD has also been split into its own file. CGI.pm now contains around
      4000 lines of code, which compared to some modules on CPAN isn't really
      that much

    - This essentially deprecates the -compile pragma and ->compile method. The
      -compile pragma will no longer do anything, whereas the ->compile method
      will raise a deprecation warning. More importantly this also REMOVES the
      -any pragma because as per the documentation this pragma needed to be
      "used with care or not at all" and allowing arbitrary HTML tags is almost
      certainly a bad idea. If you are using the -any pragma and using arbitrary
      tags (or have typo's in your code) your code will *BREAK*

    - Although this release should be back compatible (with the exception of any
      code using the -any pragma) you are encouraged to test it throughly as if
      you are doing anything out of the ordinary with CGI.pm (i.e. have bugs
      that may have been masked by the AUTOLOAD feature) you may see some issues.

    - References: GH #162, GH #137, GH #164

    [ SPEC / BUG FIXES ]
    - make the list context warning in param show the filename rather than
      the package so we have more information on exactly where the warning
      has been raised from (GH #171)
    - correct self_url when PATH_INFO and SCRIPT_NAME are the same but we
      are not running under IIS (GH #176)
    - Add the multi_param method to :cgi export (thanks to xblitz for the patch
      and tests. GH #167)
    - Fix warning for lack of HTTP_USER_AGENT in CGI::Carp (GH #168)
    - Fix imports when called from CGI::Fast, restores the import of CGI functions
      into the callers namespace for users of CGI::Fast (GH leejo/cgi-fast#11 and
      GH leejo/cgi-fast#12)

    [ FEATURES ]
    - CGI::Carp now has $CGI::Carp::FULL_PATH for displaying the full path to the
      offending script in error messages
    - CGI now has env_query_string() for getting the value of QUERY_STRING from the
      environment and not that fiddled with by CGI.pm (which is what query_string()
      does) (GH #161)
    - CGI::ENCODE_ENTITIES var added to control which chracters are encoded by the
      call to the HTML::Entities module - defaults to &<>"\x8b\x9b' (GH #157)

    [ DOCUMENTATION ]
    - Fix some typos (GH #173, GH #174)
    - All *documentation* for HTML functionality in CGI has been moved into
      its own namespace: CGI::HTML::Functions - although the functionality
      continues to exist within CGI.pm so there are no code changes required
      (GH #142)
    - Add missing documentation for env variable fetching routines (GH #163)

    [ TESTING ]
    - Increase test coverage (GH #3)

    [ INTERNALS ]
    - Cwd made a TEST_REQUIRES rather than a BUILD_REQUIRES in Makefile.PL
      (GH #170)
    - AutoloadClass variables have been removed as AUTOLOAD was removed in
      v4.14 so these are no longer necessary (GH #172 thanks to alexmv)
    - Remove dependency on constant - internal DEBUG, XHTML_DTD and EBCDIC
      constants changes to $_DEBUG, $_XHTML_DTD, and $_EBCDIC
jsonn pushed a commit to jsonn/pkgsrc that referenced this issue May 31, 2015
Update to 4.20:
fb0baab 
4.20 2015-05-29

    [ RELEASE NOTES ]
    - CGI.pm is now considered "done". See also "mature" and "legacy"
      Features requests and none critical issues will be outright rejected.
      The module is now in maintenance mode for critical issues only.

    - This release removes the AUTOLOAD and compile optimisations from CGI.pm
      that were introduced into CGI.pm twenty (20) years ago as a response to
      its large size, which meant there was a significant compile time penalty.

    - This optimisation is no longer relevant and makes the code difficult to
      deal with as well as making test coverage metrics incorrect. Benchmarks
      show that advantages of AUTOLOAD / lazy loading / deferred compile are
      less than 0.05s, which will be dwarfed by just about any meaningful code
      in a cgi script. If this is an issue for you then you should look at
      running CGI.pm in a persistent environment (FCGI, etc)

    - To offset some of the time added by removing the AUTOLOAD functionality
      the dependencies have been made runtime rather than compile time. The
      POD has also been split into its own file. CGI.pm now contains around
      4000 lines of code, which compared to some modules on CPAN isn't really
      that much

    - This essentially deprecates the -compile pragma and ->compile method. The
      -compile pragma will no longer do anything, whereas the ->compile method
      will raise a deprecation warning. More importantly this also REMOVES the
      -any pragma because as per the documentation this pragma needed to be
      "used with care or not at all" and allowing arbitrary HTML tags is almost
      certainly a bad idea. If you are using the -any pragma and using arbitrary
      tags (or have typo's in your code) your code will *BREAK*

    - Although this release should be back compatible (with the exception of any
      code using the -any pragma) you are encouraged to test it throughly as if
      you are doing anything out of the ordinary with CGI.pm (i.e. have bugs
      that may have been masked by the AUTOLOAD feature) you may see some issues.

    - References: GH #162, GH #137, GH #164

    [ SPEC / BUG FIXES ]
    - make the list context warning in param show the filename rather than
      the package so we have more information on exactly where the warning
      has been raised from (GH #171)
    - correct self_url when PATH_INFO and SCRIPT_NAME are the same but we
      are not running under IIS (GH #176)
    - Add the multi_param method to :cgi export (thanks to xblitz for the patch
      and tests. GH #167)
    - Fix warning for lack of HTTP_USER_AGENT in CGI::Carp (GH #168)
    - Fix imports when called from CGI::Fast, restores the import of CGI functions
      into the callers namespace for users of CGI::Fast (GH leejo/cgi-fast#11 and
      GH leejo/cgi-fast#12)

    [ FEATURES ]
    - CGI::Carp now has $CGI::Carp::FULL_PATH for displaying the full path to the
      offending script in error messages
    - CGI now has env_query_string() for getting the value of QUERY_STRING from
      the environment and not that fiddled with by CGI.pm (which is what
      query_string() does) (GH #161)
    - CGI::ENCODE_ENTITIES var added to control which chracters are encoded by
      the call to the HTML::Entities module - defaults to &<>"' (GH #157 - the
      \x8b and \x9b chars have been removed from this list as we are concerned
      more about unicode compat these days than old browser support.)

    [ DOCUMENTATION ]
    - Fix some typos (GH #173, GH #174)
    - All *documentation* for HTML functionality in CGI has been moved into
      its own namespace: CGI::HTML::Functions - although the functionality
      continues to exist within CGI.pm so there are no code changes required
      (GH #142)
    - Add missing documentation for env variable fetching routines (GH #163)

    [ TESTING ]
    - Increase test coverage (GH #3)

    [ INTERNALS ]
    - Cwd made a TEST_REQUIRES rather than a BUILD_REQUIRES in Makefile.PL
      (GH #170)
    - AutoloadClass variables have been removed as AUTOLOAD was removed in
      v4.14 so these are no longer necessary (GH #172 thanks to alexmv)
    - Remove dependency on constant - internal DEBUG, XHTML_DTD and EBCDIC
      constants changes to $_DEBUG, $_XHTML_DTD, and $_EBCDIC
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation Unimportant
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@leejo @ache