Requesting freeze exception for cwidget
Hi,
I'd like to request a freeze exemption for the cwidget library so
that 0.5.12-3 can replace 0.5.12-1.
The 0.5.12-3 upload fixes a nasty string-handling bug in "swsprintf",
a safe sprintf replacement that's used all over cwidget and aptitude.
As far as I know the impact is limited to adding garbage at the end of
long output strings, but it's already caused problems for users of
aptitude in the Russian locale. (#496119)
I also fixed ssprintf in some cases where it failed to call va_end();
from the manpage, I think this was at least a potential memory leak.
I've attached a debdiff between the two versions.
--- cwidget-0.5.12/debian/control
+++ cwidget-0.5.12/debian/control
@@ -2,7 +2,7 @@
Priority: extra
Section: libs
Homepage: http://cwidget.alioth.debian.org
-Vcs-Git: git://git.debian.org/cwidget/debian
+Vcs-Git: git://git.debian.org/git/cwidget/debian
Vcs-Browser: http://git.debian.org/?p=cwidget/debian/.git;a=summary
Maintainer: Daniel Burrows <dburrows@debian.org>
Build-Depends: debhelper (>= 5.0.0), libsigc++-2.0-dev,
This fixes the URL to the Git repository.
--- cwidget-0.5.12.orig/tests/test_ssprintf.cc
+++ cwidget-0.5.12/tests/test_ssprintf.cc
[snip]
A test case has been added to verify that swsprintf can correctly
generate long output strings and empty output strings:
+ void test_swsprintf()
+ {
+ // Test that inserting very long strings via ssprintf actually works.
+ std::wstring horriblelongthing = L"abcdefghijklmnopqrstuvwxyz";
+ while(horriblelongthing.size() < 4096)
+ horriblelongthing += horriblelongthing;
+
+ CPPUNIT_ASSERT_EQUAL(horriblelongthing + L" 20", swsprintf(L"%ls %d", horriblelongthing.c_str(), 20));
+
+ // Test that we can generate empty strings.
+ CPPUNIT_ASSERT_EQUAL(std::wstring(), swsprintf(L"%s", ""));
+ }
--- cwidget-0.5.12.orig/src/cwidget/generic/util/ssprintf.cc
+++ cwidget-0.5.12/src/cwidget/generic/util/ssprintf.cc
@@ -51,7 +51,10 @@
const int amt = vsnprintf(buf, initbufsize, format, ap);
if(amt < initbufsize)
- return buf;
+ {
+ va_end(ap2);
+ return buf;
+ }
else
{
const int buf2size = amt + 1;
Call va_end() on a copied argument list before returning.
@@ -64,6 +67,7 @@
string rval(buf2, amt2);
delete[] buf2;
+ va_end(ap2);
return rval;
}
The same.
The last hunk is essentially a complete rewrite of the vswsprintf
routine. For your convenience, here's the new code:
wstring vswsprintf(const wchar_t *format, va_list ap)
{
// Unlike sprintf, swsprintf just returns -1 when there's an
// overflow. So we have to keep making bigger buffers until we
// have one that's big enough.
bool ok = false;
wstring rval;
int bufsize = initbufsize;
while(!ok)
{
va_list ap2;
va_copy(ap2, ap);
wchar_t *buf = new wchar_t[bufsize];
int amt = vswprintf(buf, bufsize, format, ap2);
if(amt >= 0 && amt < bufsize)
{
rval = buf;
ok = true;
}
else
bufsize *= 2;
delete[] buf;
va_end(ap2);
}
return rval;
}
The old version looked like this:
wstring vswsprintf(const wchar_t *format, va_list ap)
{
wchar_t buf[initbufsize];
int amt = vswprintf(buf, initbufsize, format, ap);
if(amt < initbufsize)
return buf;
else
{
wchar_t *buf2 = new wchar_t[amt+1];
int amt2 = vswprintf(buf2, initbufsize, format, ap);
eassert(amt2 < amt+1);
wstring rval(buf2, amt2);
delete[] buf2;
return rval;
}
}
The basic problem was that I somehow got the idea that vswprintf
worked like vsprintf, which returns the desired buffer size if there
wasn't enough room. No such luck. vswprintf actually returns -1 when
it fails; the only way to find the right buffer size is to try again
with a bigger buffer. The old code was, as you can see, just returning
immediately with a partially filled buffer; since the buffer didn't get
null-terminated, that had the effect of returning a bunch of garbage
concatenated onto the end of a truncated string at the 512-byte mark.
Thanks,
Daniel
diff -u cwidget-0.5.12/debian/changelog cwidget-0.5.12/debian/changelog
--- cwidget-0.5.12/debian/changelog
+++ cwidget-0.5.12/debian/changelog
@@ -1,3 +1,11 @@
+cwidget (0.5.12-2) unstable; urgency=low
+
+ * Backport a fix for a string truncation bug from HEAD (Closes: #496119).
+
+ * Fix the URL to the packaging VCS (Closes: #492584).
+
+ -- Daniel Burrows <dburrows@debian.org> Sun, 31 Aug 2008 22:01:53 -0700
+
cwidget (0.5.12-1) unstable; urgency=low
* New upstream version.
diff -u cwidget-0.5.12/debian/control cwidget-0.5.12/debian/control
--- cwidget-0.5.12/debian/control
+++ cwidget-0.5.12/debian/control
@@ -2,7 +2,7 @@
Priority: extra
Section: libs
Homepage: http://cwidget.alioth.debian.org
-Vcs-Git: git://git.debian.org/cwidget/debian
+Vcs-Git: git://git.debian.org/git/cwidget/debian
Vcs-Browser: http://git.debian.org/?p=cwidget/debian/.git;a=summary
Maintainer: Daniel Burrows <dburrows@debian.org>
Build-Depends: debhelper (>= 5.0.0), libsigc++-2.0-dev,
only in patch2:
unchanged:
--- cwidget-0.5.12.orig/tests/test_ssprintf.cc
+++ cwidget-0.5.12/tests/test_ssprintf.cc
@@ -1,6 +1,6 @@
// Tests for generic/util/ssprintf.
//
-// Copyright (C) 2007 Daniel Burrows
+// Copyright (C) 2007-2008 Daniel Burrows
//
// This program is free software; you can redistribute it and/or
// modify it under the terms of the GNU General Public License as
@@ -19,12 +19,36 @@
#include <cppunit/extensions/HelperMacros.h>
+#include <cppunit/TestAssert.h>
+
#include <cwidget/generic/util/ssprintf.h>
+// For displaying assertion failures.
+#include <cwidget/generic/util/transcode.h>
+
#include <errno.h>
#include <string.h>
using cwidget::util::ssprintf;
+using cwidget::util::swsprintf;
+
+CPPUNIT_NS_BEGIN
+
+template <>
+struct assertion_traits<std::wstring>
+{
+ static bool equal(const std::wstring &x, const std::wstring &y)
+ {
+ return x == y;
+ }
+
+ static std::string toString(const std::wstring &x)
+ {
+ return cwidget::util::transcode(x);
+ }
+};
+
+CPPUNIT_NS_END
class SSPrintfTest : public CppUnit::TestFixture
{
@@ -32,6 +56,7 @@
CPPUNIT_TEST(test_sstrerror);
CPPUNIT_TEST(test_ssprintf);
+ CPPUNIT_TEST(test_swsprintf);
CPPUNIT_TEST_SUITE_END();
private:
@@ -62,6 +87,16 @@
CPPUNIT_ASSERT_EQUAL(horriblelongthing + " 20", ssprintf("%s %d", horriblelongthing.c_str(), 20));
}
+
+ void test_swsprintf()
+ {
+ // Test that inserting very long strings via ssprintf actually works.
+ std::wstring horriblelongthing = L"abcdefghijklmnopqrstuvwxyz";
+ while(horriblelongthing.size() < 4096)
+ horriblelongthing += horriblelongthing;
+
+ CPPUNIT_ASSERT_EQUAL(horriblelongthing + L" 20", swsprintf(L"%ls %d", horriblelongthing.c_str(), 20));
+ }
};
CPPUNIT_TEST_SUITE_REGISTRATION(SSPrintfTest);
only in patch2:
unchanged:
--- cwidget-0.5.12.orig/src/cwidget/generic/util/ssprintf.cc
+++ cwidget-0.5.12/src/cwidget/generic/util/ssprintf.cc
@@ -51,7 +51,10 @@
const int amt = vsnprintf(buf, initbufsize, format, ap);
if(amt < initbufsize)
- return buf;
+ {
+ va_end(ap2);
+ return buf;
+ }
else
{
const int buf2size = amt + 1;
@@ -64,6 +67,7 @@
string rval(buf2, amt2);
delete[] buf2;
+ va_end(ap2);
return rval;
}
@@ -82,25 +86,38 @@
wstring vswsprintf(const wchar_t *format, va_list ap)
{
- wchar_t buf[initbufsize];
- int amt = vswprintf(buf, initbufsize, format, ap);
+ // Unlike sprintf, swsprintf just returns -1 when there's an
+ // overflow. So we have to keep making bigger buffers until we
+ // have one that's big enough.
- if(amt < initbufsize)
- return buf;
- else
+ bool ok = false;
+
+ wstring rval;
+ int bufsize = initbufsize;
+
+ while(!ok)
{
- wchar_t *buf2 = new wchar_t[amt+1];
+ va_list ap2;
+ va_copy(ap2, ap);
- int amt2 = vswprintf(buf2, initbufsize, format, ap);
+ wchar_t *buf = new wchar_t[bufsize];
- eassert(amt2 < amt+1);
+ int amt = vswprintf(buf, bufsize, format, ap2);
- wstring rval(buf2, amt2);
+ if(amt > 0 && amt < bufsize)
+ {
+ rval = buf;
+ ok = true;
+ }
+ else
+ bufsize *= 2;
- delete[] buf2;
+ delete[] buf;
- return rval;
+ va_end(ap2);
}
+
+ return rval;
}
// There are two variants of strerror_r to watch out for. The GNU
Reply to: