Re: [PATCH v2 3/4] userdiff: fix typo in ruby and python word regexes

[Thomas Rast <trast@student.ethz.ch>]

Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > Both had an unclosed ] that ruined the safeguard against not matching
> > a non-space char.
> 
> Thanks.
> 
> Couldn't we have found this without your "sanity check" patch?  Are we
> ignoring error returns from regcomp() in some codepath, or is it just that
> we are catching them but our test suite lacks ruby and python test
> vectors?

We lacked test vectors, but we still couldn't have caught it.  We do
check for errors in regcomp():

	if (o->word_regex) {
		ecbdata.diff_words->word_regex = (regex_t *)
			xmalloc(sizeof(regex_t));
		if (regcomp(ecbdata.diff_words->word_regex,
				o->word_regex,
				REG_EXTENDED | REG_NEWLINE))
			die ("Invalid regular expression: %s",
					o->word_regex);
	}

(Now that I'm seeing this and comparing with regcomp(3), we should
actually report regerror() as part of the error message.)

The problem is that the pattern is still valid.  Consider that it was
a final two arms to the regex:

-        "|[^[:space:]|[\x80-\xff]+"),
+        "|[^[:space:]]|[\x80-\xff]+"),

In the preimage, it parses like so:

  | [^
      [:space:]|[\x80-\xff
     ]+

That is, the third [ is part of the (negated) character class.  So the
only problem is with | or [ characters in the input.  Any other
non-space character is part of the class.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch