[PATCH] Add stricter checks when parsing incoming patches

[Stephen Finucane]

On Fri, 2018-07-13 at 14:46 -0400, Don Zickus wrote:
> The patch parser has a multi-stage if-then-else statement that tries to
> determine which part of a patch is the comment and what piece is the
> patch itself.
> 
> Unfortunately there is a gap in between state 1 and state 2 where chunks
> of the comment can be accidentally added to the patch.
> 
> For example if a comment has a line that begins with 'diff' or 'Index'.
> That will trigger the state 0 to state 1 transition and sit there until
> many comment lines later a '--- ' line is found to move it to state 2.
> 
> As a result many comment lines are truncated and stuck into a patch buffer
> instead.  This makes it more difficult to process metadata found in the
> comment buffer.
> 
> This patch adds some strict rules based on various common patch preambles
> like git, quilt, and rcs.
> 
> Now if the patch is in state 1 because of a 'diff ' or 'Index:', it needs
> to expect the next common preamble to continue to stay in state 1 otherwise
> accept the state 0 to state 1 transition was an accident and move back to
> state 0.
> 
> This patch has been in our internal patchwork instance for 8 years now and is
> the result of various patches we have seen internally.
> 
> Signed-off-by: Don Zickus <dzickus at redhat.com>

I applied this locally and ran the unit tests.

  $ docker-compose run --rm web tox -e py27-django111

It's failing pretty hard.

  Ran 455 tests in 145.925s

  FAILED (failures=62, errors=6, skipped=1)
  Destroying test database for alias 'default'...

I'm guessing a good few of these are as a result of how we auto-
generate patches but I imagine a few more of them might be valid. Could
you have a look at these failures and see how we can remedy them?

Stephen

> ---
>  patchwork/parser.py | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/patchwork/parser.py b/patchwork/parser.py
> index a2db403..5566c69 100644
> --- a/patchwork/parser.py
> +++ b/patchwork/parser.py
> @@ -792,6 +792,19 @@ def parse_patch(content):
>  
>              if line.startswith(('rename from ', 'rename to ')):
>                  state = 6
> +            elif line.startswith('diff ') or line.startswith('Index: ') \
> +                    or line.startswith('deleted file ') \
> +                    or line.startswith('index ') \
> +                    or line.startswith('new file ') \
> +                    or line.startswith('====') \
> +                    or line.startswith('RCS file: ') \
> +                    or line.startswith('retrieving revision '):
> +                state = 1
> +            else:
> +                state = 0
> +                commentbuf += buf
> +                buf = ''
> +
>          elif state == 2:
>              if line.startswith('+++ '):
>                  state = 3