Pylons/peppercorn — pull request #5 by vung (2012-02-05 16:50:41+00:00)
Title: Stricter handling of input 

This is a set of changes to the way input is handled in Peppercorn.

In short:
1. Use a single, module-specific, exception type for parsing errors and document that `parse()` might raise an exception.
2. Eliminate use of recursion and either parse successfuly the whole input or raise an exception

The recommended usage doesn't mention that `parse()` might raise exceptions.
The three places [1] I saw using `parse()` assume it returns sucessfully, so it's easy to feed input to a form that ultimately results in a server error.
I don't know if people really expected it to not raise an exception or simply followed the documented example.

The test suite already accounts for `ValueError`. Aditionally, `peppercorn.parse()` might raise:

1. `RuntimeError`, when the input (even a well-formed one) is nested deeper than the recursion limit
2. `StopIteration` when a sequence is started and not closed.

Presumably the correct usage would be:

try:
pstruct = peppercorn.parse(fields)
except (RuntimeError, ValueError, StopIteration):
# malformed syntax
raise HTTPBadRequest()

It assumes access to a framework-specific exception, though.

The first commit establishes a `ParseError`, which hopefully makes it easier to target exceptions raised by `peppercorn.parse()`.

The second commit takes the documentation at face value ("Mappings and sequences can be nested arbitrarily") and makes it so. This is done by replacing the recursive implementation with one that uses a list as a stack.

Doing this brought on another thing: `peppercorn.parse()` handles input that contains extra end markers by simply returning what was collected up to that point.

The non-recursive implementation passed the full test suite even if (as I discoverd later) it handled extra end markers differently.

I added some tests and contorted it a bit to preserve the original behaviour.

I still think that an input like `[('foo', 'fred'), ('__end__', ''), ('bar', 'joe')]` should raise an exception and not be parsed as `{'foo': 'fred'}`.

This is done in the last commit. I separated the \__end__ tests as they might be useful even if you disagree.

[1]
https://github.com/Pylons/deform/blob/master/deform/field.py#L502
https://bitbucket.org/danjac/pyramid_simpleform/src/32d81533f51b/pyramid_simpleform/form.py#cl-107
https://bitbucket.org/ianjosephwilson/pepperedform/src/10b60a55c737/pepperedform/handler.py#cl-16