lkml.org 
[lkml]   [2017]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] checkpatch: Flag spam header (X-Spam-Report) to prevent spurious warnings
From
Date
On Wed, 2017-03-22 at 08:25 -0700, Darren Hart wrote:
> On Tue, Mar 21, 2017 at 11:31:08AM -0700, Joe Perches wrote:
> > On Tue, 2017-03-21 at 09:30 -0700, John 'Warthog9' Hawley (VMware) wrote:
> > > Spamassassin sticks a long (~79 character) long string after a
> > > line that has a single space in it. The line with space causes
> > > checkpatch to erroniously think that it's in the content body, as
> > > opposed to headers and thus flag a mail header as an unwrapped long
> > > comment line.
> >
> > If the spammassassin header is like
> >
> > email-header-n: foo
> > email-header-m: bar
> >
> > X-Spam-Report: bar
>
> The specific content of the X-Spam-Report that triggers this for me,
> from this patch for example, is:
>
> === 8< ===
> X-Spam-Report: SpamAssassin version 3.4.1 on casper.infradead.org summary:
> Content analysis details: (-1.9 points, 5.0 required)
>
> pts rule name description
> ---- ---------------------- --------------------------------------------------
> -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1%
> [score: 0.0000]
> X-TUID: alGBIuPZmqOj
>
> === >8 ===
>
> The long ---- ----... line is over 75 characters and triggers the test
> for long commit_log lines.
>
> >
> > Does that form follow rfc 5322?
>
> By my reading, this is governed by the long header fields defined by
> 2.2.3, with whitespace folding defined as "a CRLF may be inserted before
> any WSP."
>
> >
> > If it does then any email header could have that
> > form and the header wrapping test should be
>
> Yes, agreed.
>
> So the logic we want is:
>
> If we are in headers and we detect a CRLF and the next line starts with a WSP,
> then we are still in headers (and therefor not in the commit log). The CRLF
> information does not appear to be available as it is replaced with just \n.
>
> > updated from
> >
> > if ($in_header_lines && $realfile =~ /^$/ &&
> >     !($rawline =~ /^\s+\S/ ||
> >       $rawline =~ /^(commit\b|from\b|[\w-]+:).*$/i)) {
> > $in_header_lines = 0;
> > $in_commit_log = 1;
> > $has_commit_log = 1;
> > }
> >
> > to something like
> >
> > if ($in_header_lines && $realfile =~ /^$/ &&
> >     !($rawline =~ /^ (?:\s*\S|$)/ ||
>
> Hrm... lines that start with maybe a space followed by a : ... Why did you
> introduce that part of the check?

The regex doesn't care about colons.
It's a perl non-capturing group.
https://perldoc.perl.org/perlretut.html#Non-capturing-groupings

> Looking at this more closely, I was also not clear why the original test looked
> for several spaces followed by non-space. What case is this for?

Not several spaces, one or more spaces then a non-space.
The only change here is allowing an initial space followed
by either:

1: optional spaces, then non-space.
2: EOL

I supposed you could argue that case 2 should
also allow optional spaces before EOL and the
test should be

if ($in_header_lines && $realfile =~ /^$/ &&
    !($rawline =~ /^\s+(?:\S|$)/ ||
...

\
 
 \ /
  Last update: 2017-03-22 19:18    [W:0.076 / U:0.788 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site