[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug-diffutils] bug#35256: Bug report for -W argument (maximum width) -

From: alec
Subject: [bug-diffutils] bug#35256: Bug report for -W argument (maximum width) - minor and not dangerous
Date: Sat, 13 Apr 2019 12:29:45 +0100

Hello there.

I was hoping to view a side-by-side diff of something and, perhaps unfairly, was hoping for a setting where diff would choose a width such that there were no truncations and I would use less with no wrapping to inspect the results.

My first attempt was "-W 0" (a width of 0 has no "legit" meaning afterall) - error, so I tried -1. This leads to a weird situation where it seems to just output loads of tabs - while it'll probably still terminate eventually the behaviour is unreasonable.

To try this yourself run something like:

diff -y ./maps ./task/4974/maps -W -1

from /proc/XXXX where XXXX is some PID for a program with threads (eg firefox) and the 4974 is any task that isn't XXXX

ADDENDUM: The -1 isn't important, 9999999999999 also illustrates the problem - END ADDENDUM

Looking at the code (in the 3.7 tarball, src/diff.c modified on 18th of December 2018) notice:

Line 284:
  uintmax_t numval;
Line 525:
    case 'W':
      numval = strtoumax (optarg, &numend, 10);
      if (! (0 < numval && numval <= SIZE_MAX) || *numend)
        try_help ("invalid width '%s'", optarg);
      if (width != numval)
          if (width)
        fatal ("conflicting width options");
          width = numval;

For convenience:
      uintmax_t strtoumax(const char *nptr, char **endptr, int base);
and it may set errno, my man page doesn't say whether this -1 behaviour is "okay" however it probably is, unsigned afterall, this means that numval is going to be a really really big value.


Just basic DOS (denial-of-service) stuff, a CPU usage spike comes from diff itself and it seems to output a lot of tabs (a good 275mib / sec on my machine) and will probably do so for a good few years before anything else comes out, a testament to the robustness of diff is that it did this, and its memory usage didn't start ballooning.

I know diff is used by A LOT of other programs, some of which are web-accessible (eg mediawiki uses diff - and will by default if it finds it), many of my projects use it too. It is not a big stretch to imagine someone has a web-service out there which allows side-by-side format, and not much of a further leap to assume that someone might have an input box for width which exposes -W, guarded only by a regex of the form ^[1-9][0-9]* (which yes, wont allow -1 but will allow 9999999999999)

You could bring a server to its knees pretty quickly using just diff's CPU usage and a few tabs using this - that's not even considering whether or not the system hypothesised here doesn't have trouble with memory from a convenient get_line() function first.

While not really diff's fault or problem, a potential solution detailed below would fix it and not cause any problems for those with legit (?) needs for really wide diffs

Humans are limiting here, improvements and the growth of computers wont really affect the maximum width so putting a limit in place is reasonable. I make no claim there is a "maximum useful width" so being able to override will ensure my half-assed musings on such a limit wont cause any problems in the future.

I'd go with something like

Add a check that numval is <= get_reasonable_specified_width_limit() after the existing checks, if not output an error in the form of:

"You probably don't want to do that, see [wherever], if you do specify --we-have-evolved-cylindical-lenses-now or set the environmental variable GNU_DIFF_REASONABLE_LIMIT to a new limit, using 0 for none"

Lastly, for what it's worth from a perfect stranger:

I'm very impressed that diff didn't start consuming huge amounts of memory, and a little saddened that it is impressive!

Thanks very much for diff and your work on it, you have no idea how many things it underpins!

reply via email to

[Prev in Thread] Current Thread [Next in Thread]