Zope icon indicating copy to clipboard operation
Zope copied to clipboard

Modernize request parameter handling

Open d-maurer opened this issue 6 years ago • 2 comments

Zope's current request parameter handling has several known bugs/caveats:

  1. There is almost no error handling:

    • unrecognised directives are silently ignored
    • if a request parameter contains several converter directives, the leftmost wins
    • if a request parameter contains several encoding directives, the leftmost wins
    • if a request parameter contains an encoding but no converter directive, the encoding directive is silently ignored
    • some directive combinations do not make sense (e.g. :record:records); for them, some of the directives are silently ignored
  2. Usually, the order of aggregator directives in a request parameter does not matter. However, this is not the case for the :tuple directive. To really produce a tuple request variable, it must be the left most directive; otherwise, it is equivalent to :list.

    In addition, :tuple is always equivalent to :list for request variables aggregated as record or sequence of records.

  3. The main use case for the :default directive is to provide a default value for form controls (e.g. checkboxes) for which the browser may or may not pass on a value when the form is submitted. Unfortunately, this only works at the top level. It does not work for subcomponents, e.g. an attribute of a "record". As a consequence, if a request parameter combines :default with another aggregator directive, the result may be unexpected.

  4. The request preprocessing happens at a very early stage, before traversal has taken place. As a consequence, important configuration for application specific error handling may not yet have taken effect. Exceptions raised during this stage are reported and tracked only via "root level" error handling.

In addition, for Python 3, Zope's handling of encoding directives is currently broken (--> #634) and converter support is partially broken (#638, #558). Furthermore, Zope's encoding support is not in line with modern standards, e.g. HTML5 form support.

This issue proposes a major overhaul of Zope's request parameter handling:

  • either drop support for the zpublisher-default-encoding configuration option and fix the encoding to utf-8 (this seems to be in line with current standards, see e.g. HTML living standard; they seem to view any non-utf-8 encoding as "legacy support" only) or fully support HTML5' form encoding practices.
  • replace the current (apparently ad hoc) handling of aggregator directives by a consistent, orthogonal, fully recursive implementation with clear semantics (thus, hopefully, avoiding surprises as with the current :default and :tuple aggregator)
  • overhaul the implementation for "converter" and "encoding" directives; in particular ensure that for Python 3, the "u*" converter directives behave as the converter directive without the "u" prefix; view the "encoding" as parameter for the "converter"
  • fully recognize errors but delay error reporting by registering an appropriate "PostAuthenticationHook"; this way errors can in many cases (when the error does not affect the following traversal) be reported in the correct application context.
  • support the special handling of HTML's input controls of type image.
  • (maybe) support jQuery's "modern" parameter serialization (apparently initially invented by PHP and now also use by Ruby on rails): i.e. var[] is equivalent to var:list and var[attr] to var.attr:record.

d-maurer avatar May 28 '19 07:05 d-maurer

@d-maurer wrote: […]

This issue proposes a major overhaul of Zope's request parameter handling:

First of all, I personally do not use Zope's request parameters, but I see the issues and the need for them to be fixed.

  • either drop support for the zpublisher-default-encoding configuration option and fix the encoding to utf-8 (this seems to be in line with current standards, see e.g. HTML living standard; they seem to view any non-utf-8 encoding as "legacy support" only) or fully support HTML5' form encoding practices.

Dropping zpublisher-default-encoding configuration option would make it for older code using latin1 though out the whole application more difficult to migrate. So I'd like to keep it as it is.

  • replace the current (apparently ad hoc) handling of aggregator directives by a consistent, orthogonal, fully recursive implementation with clear semantics (thus, hopefully, avoiding surprises as with the current :default and :tuple aggregator)

+1

  • overhaul the implementation for "converter" and "encoding" directives; in particular ensure that for Python 3, the "u*" converter directives behave as the converter directive without the "u" prefix; view the "encoding" as parameter for the "converter"

+1

  • fully recognize errors but delay error reporting by registering an appropriate "PostAuthenticationHook"; this way errors can in many cases (when the error does not affect the following traversal) be reported in the correct application context.

+1

  • support the special handling of HTML's input controls of type image.

good idea

  • (maybe) support jQuery's "modern" parameter serialization (apparently initially invented by PHP and now also use by Ruby on rails): i.e. var[] is equivalent to var:list and var[attr] to var.attr:record.

I am not sure if there will be someone using these new spellings, especially as the old ones should be kept.

icemac avatar Jun 12 '19 06:06 icemac

@icemac

@d-maurer wrote: ...

  • (maybe) support jQuery's "modern" parameter serialization (apparently initially invented by PHP and now also use by Ruby on rails): i.e. var[] is equivalent to var:list and var[attr] to var.attr:record.

I am not sure if there will be someone using these new spellings, especially as the old ones should be kept.

It is used by jQuery's params (unless you specify traditional mode) when you call it with a JS object containing more complex values (such as lists or subobjects). But, you are right. It would make an already complex implementation even more complex.

d-maurer avatar Jun 12 '19 06:06 d-maurer