buglet (?) in qp-decode.c

[Ralf Fassel]

When a quoted-printable message is incorrectly terminated at EOF:

p;</td>=0A=0A <td width=3D"49%">&nbsp;</td>=0A </tr>=0A </tb=
ody>=0A </table>=0A</body>=0A</html>=0A=
--=_mixed 002D08B5C1257475_=--

(missing newline before the separator) qp-decode barfs:

Error: line 758: '^@' is something other than line break or hex digit after = in quoted-printable encoding

and continues to read into the invalid buffer contents.

IMHO, if EOF is encountered, it simply should stop processing and
issue no error message:

*** src/qp-decode.c~ Wed Feb 20 22:11:51 2008
--- src/qp-decode.c Mon Jun 30 12:03:41 2008
***************
*** 88,93 ****
--- 88,94 ----
for (stop++; *stop && (*stop == ' ' || *stop == '\t'); stop++)
;
} else {
+ if (feof(stdin)) exit(0);
fprintf(stderr, "Error: line %d: '%c' is something other than line break or hex digit after = in quoted-printable encoding\n", lineno, *stop);
putchar('=');
putchar(*stop);

R'

[blueman]

I think I had noticed and reported a similar issue about 18 months
ago. Robert also submitted a patch which I thought was now part of the
standard VM distribution (but maybe not).

You may want to go back in the archives:
http://groups.google.com/group/gnu.emacs.vm.info/browse_thread/thread/5b30c79c9137e55a/9b1f53c459d6c915?lnk=st&q=blueman++qp-decode#
http://groups.google.com/group/gnu.emacs.vm.bug/browse_thread/thread/7b3640b3291cc9bc

[blueman]

I think I had noticed and reported a similar issue about 18 months

[blueman]

I think I had noticed and reported a similar issue about 18 months

[Ralf Fassel]

* blueman <NOS...@nospam.com>

| > *** src/qp-decode.c~ Wed Feb 20 22:11:51 2008
| > --- src/qp-decode.c Mon Jun 30 12:03:41 2008
| > ***************
| > *** 88,93 ****
| > --- 88,94 ----
| > for (stop++; *stop && (*stop == ' ' || *stop == '\t'); stop++)
| > ;
| > } else {
| > + if (feof(stdin)) exit(0);
| > fprintf(stderr, "Error: line %d: '%c' is something other than line break or hex digit after = in quoted-printable encoding\n", lineno, *stop);
| > putchar('=');
| > putchar(*stop);
|

| I think I had noticed and reported a similar issue about 18 months
| ago. Robert also submitted a patch which I thought was now part of the
| standard VM distribution (but maybe not).

I think that your patch was about *not* exit(1)ing when this error
occurs due to an actual encoding error in the buffer (there actually
is another char at *stop). In my case the char at *stop is the
terminating '\0' character which was placed in the buffer by fgets().

At least the exit(1) in this part of the code (see below, marked (2))
is commented out in the latest VM release, where it was still active
in prior versions (I still use 7.19).

Not exiting here on EOF is a definite bug, since there is nothing more
to read in the buffer, but the code continues anyway (stop++).

Maybe the char which 'stop' is pointing at should be considered here,
and if it is '\0', then the end of the fgets-buffer has been reached.
There shouldn't be any binary content in the buffer, so '\0' cannot
occur there except when added by fgets as terminating character.

The code branch in question is (abbreviated):

} else { /* *stop == '=' */
stop++;
if ((d1 = strchr(hexdigits, *(stop))) &&
(d2 = strchr(hexdigits, *(stop+1)))) {
...
} else if ((d1 = strchr(hexdigits2, *(stop))) &&
(d2 = strchr(hexdigits2, *(stop+1)))) {
...
} else if (*stop == '\n') {
...
} else if (*stop == '\r') {
...
} else if (*stop == ' ' || *stop == '\t') {
...
} else {
(1)

fprintf(stderr, "Error: line %d: '%c' is something other than line break or hex digit after = in quoted-printable encoding\n", lineno, *stop);
putchar('=');
putchar(*stop);

stop++;
(2) /* exit(1); */
}
start = stop;
goto keep_processing;
}

Maybe at (1) another check for
else if (*stop == '\0')
needs to be added instead or in addition of the feof() check.

R'

[newsspam5...@robf.de]

Ralf Fassel <ral...@gmx.de> writes:
[...]

> Maybe at (1) another check for
> else if (*stop == '\0')
> needs to be added instead or in addition of the feof() check.

Does the following work for you?

Robert

=== modified file 'src/qp-decode.c'
--- src/qp-decode.c 2007-01-12 01:50:26 +0000
+++ src/qp-decode.c 2008-07-05 21:23:47 +0000
@@ -63,7 +63,9 @@
continue;

} else { /* *stop == '=' */
stop++;

- if ((d1 = strchr(hexdigits, *(stop))) &&
+ if (*stop == 0) {
+ continue;
+ } else if ((d1 = strchr(hexdigits, *(stop))) &&
(d2 = strchr(hexdigits, *(stop+1)))) {
c = (d1 - hexdigits) * 16 + (d2 - hexdigits);
putchar(c);

[Ralf Fassel]

* newsspam5...@robf.de

| Does the following work for you?

--<snip-snip>--

| --- src/qp-decode.c 2007-01-12 01:50:26 +0000
| +++ src/qp-decode.c 2008-07-05 21:23:47 +0000
| @@ -63,7 +63,9 @@
| continue;
| } else { /* *stop == '=' */
| stop++;
| - if ((d1 = strchr(hexdigits, *(stop))) &&
| + if (*stop == 0) {
| + continue;
| + } else if ((d1 = strchr(hexdigits, *(stop))) &&
| (d2 = strchr(hexdigits, *(stop+1)))) {
| c = (d1 - hexdigits) * 16 + (d2 - hexdigits);
| putchar(c);

Yes, it does.

R'

[newsspam5...@robf.de]

Thanks for testing.

Robert.