Control: tags -1 + patch
On Sat, 28 Apr 2018 at 20:59:16 +0900, d...@debian.org wrote:
> original bug report: https://bugs.debian.org/896596
> previous forwarded: https://github.com/ruby-gnome2/ruby-gnome2/issues/1159
>
> below comments and patch by Kouhei Sutou <k...@cozmixng.org>.
>
> > Poppler adds getLength() check at
> > https://cgit.freedesktop.org/poppler/poppler/commit/?id=a59f61641fcb36859b625749afb4561557e419f6
> > for https://bugs.freedesktop.org/show_bug.cgi?id=103552 .
> > But PopplerInputStream created by poppler-glib always returns 0 for
> > getLength():
> > https://cgit.freedesktop.org/poppler/poppler/tree/glib/poppler-document.cc#n301
Thanks for this diagnosis. It seems correct.
> > Ah, we can just use the length passed by argument:
> - str = new PopplerInputStream(stream, cancellable, 0, gFalse, 0,
> Object(objNull));
> + str = new PopplerInputStream(stream, cancellable, 0, gFalse, length,
> Object(objNull));
That doesn't actually work, because the function is documented to accept
-1 as a length (meaning "find the length automatically").
> > We'll be able to compute length by g_seekable_seek(0, G_SEEK_END) and
> > g_seekable_tell().
That's what I've done in the case where the caller used -1. The function
already requires that the stream is seekable, so it seems safe to assume
that this works.
Proposed patches attached. I've included a fairly minimal reproducer for
the bug, as a patch to the existing autopkgtest.
Also available as a merge request, here:
https://salsa.debian.org/freedesktop-team/poppler/merge_requests/1
Patch proposed upstream here:
https://gitlab.freedesktop.org/poppler/poppler/merge_requests/189
smcv
>From 739c81306bf0d58c857d6d664f63eaaa8d4c7317 Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Thu, 14 Feb 2019 09:45:52 +0000
Subject: [PATCH 1/3] Add patch to fix poppler_document_new_from_stream()
regression
Closes: #896596
---
...ate-PopplerInputStream-with-length-0.patch | 36 +++++++++++++++++++
debian/patches/series | 1 +
2 files changed, 37 insertions(+)
create mode 100644 debian/patches/glib-Don-t-create-PopplerInputStream-with-length-0.patch
diff --git a/debian/patches/glib-Don-t-create-PopplerInputStream-with-length-0.patch b/debian/patches/glib-Don-t-create-PopplerInputStream-with-length-0.patch
new file mode 100644
index 0000000..c59de03
--- /dev/null
+++ b/debian/patches/glib-Don-t-create-PopplerInputStream-with-length-0.patch
@@ -0,0 +1,36 @@
+From: Simon McVittie <s...@debian.org>
+Date: Thu, 14 Feb 2019 09:43:32 +0000
+Subject: glib: Don't create PopplerInputStream with length 0
+
+Since commit a59f6164, PopplerInputStream requires a nonzero length.
+
+Loosely based on an earlier patch by Kouhei Sutou. This version adds
+support for length == -1, which is documented to work.
+
+Bug: https://gitlab.freedesktop.org/poppler/poppler/issues/414
+Bug-Debian: https://bugs.debian.org/896596
+Forwarded: https://gitlab.freedesktop.org/poppler/poppler/merge_requests/189
+---
+ glib/poppler-document.cc | 9 ++++++++-
+ 1 file changed, 8 insertions(+), 1 deletion(-)
+
+diff --git a/glib/poppler-document.cc b/glib/poppler-document.cc
+index ed37da4c..e04c8b42 100644
+--- a/glib/poppler-document.cc
++++ b/glib/poppler-document.cc
+@@ -309,7 +309,14 @@ poppler_document_new_from_stream (GInputStream *stream,
+ }
+
+ if (stream_is_memory_buffer_or_local_file(stream)) {
+- str = new PopplerInputStream(stream, cancellable, 0, false, 0, Object(objNull));
++ if (length == (goffset)-1) {
++ if (!g_seekable_seek(G_SEEKABLE(stream), 0, G_SEEK_END, cancellable, error)) {
++ g_prefix_error(error, "Unable to determine length of stream: ");
++ return nullptr;
++ }
++ length = g_seekable_tell(G_SEEKABLE(stream));
++ }
++ str = new PopplerInputStream(stream, cancellable, 0, false, length, Object(objNull));
+ } else {
+ CachedFile *cachedFile = new CachedFile(new PopplerCachedFileLoader(stream, cancellable, length), new GooString());
+ str = new CachedFileStream(cachedFile, 0, false, cachedFile->getLength(), Object(objNull));
diff --git a/debian/patches/series b/debian/patches/series
index d3a4373..6fed9f7 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,2 @@
+glib-Don-t-create-PopplerInputStream-with-length-0.patch
#qt-visibility.diff
--
2.20.1
>From ebfd76c4664b43ba19185d2109532395724c7ff9 Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Thu, 14 Feb 2019 09:35:58 +0000
Subject: [PATCH 2/3] d/tests/glib: Add a test for #896596
---
debian/tests/glib | 2 ++
debian/tests/test-glib.c | 45 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/debian/tests/glib b/debian/tests/glib
index 7ad9ce4..5113160 100755
--- a/debian/tests/glib
+++ b/debian/tests/glib
@@ -5,3 +5,5 @@ SRCDIR=$(dirname $(realpath $0))
cd $ADTTMP
gcc -Wall -Werror -std=c99 -o poppler-glib-test $SRCDIR/test-glib.c `pkg-config --cflags --libs poppler-glib`
./poppler-glib-test "file:///usr/share/cups/data/default-testpage.pdf"
+gcc -Wall -Werror -std=c99 -o poppler-glib-test $SRCDIR/test-glib.c -DTEST_GIO `pkg-config --cflags --libs poppler-glib gio-2.0`
+./poppler-glib-test "file:///usr/share/cups/data/default-testpage.pdf"
diff --git a/debian/tests/test-glib.c b/debian/tests/test-glib.c
index 912e7a8..56c2a68 100644
--- a/debian/tests/test-glib.c
+++ b/debian/tests/test-glib.c
@@ -1,10 +1,20 @@
#include <poppler.h>
#include <stdlib.h>
+#ifdef TEST_GIO
+#include <gio/gio.h>
+#endif
+
int main(int argc, char *argv[])
{
PopplerDocument* doc;
int n_pages;
+#ifdef TEST_GIO
+ GError *error = NULL;
+ GFile *file;
+ GFileInfo *info;
+ GInputStream *istream;
+#endif
if (argc < 2)
return EXIT_FAILURE;
@@ -19,5 +29,40 @@ int main(int argc, char *argv[])
g_assert_cmpint(n_pages, > , 0);
g_object_unref(doc);
+
+#ifdef TEST_GIO
+ /* https://bugs.debian.org/896596 */
+
+ file = g_file_new_for_uri(argv[1]);
+ g_assert_nonnull(file);
+ info = g_file_query_info(file, G_FILE_ATTRIBUTE_STANDARD_SIZE, G_FILE_QUERY_INFO_NONE, NULL, &error);
+ g_assert_no_error(error);
+ g_assert_nonnull(info);
+ g_assert_cmpint(g_file_info_get_size(info), > , 0);
+
+ istream = G_INPUT_STREAM(g_file_read(file, NULL, &error));
+ g_assert_no_error(error);
+ g_assert_nonnull(istream);
+ doc = poppler_document_new_from_stream(istream, -1, NULL, NULL, &error);
+ g_assert_no_error(error);
+ g_assert_nonnull(doc);
+ g_assert_cmpint(poppler_document_get_n_pages(doc), == , n_pages);
+ g_object_unref(doc);
+ g_object_unref(istream);
+
+ istream = G_INPUT_STREAM(g_file_read(file, NULL, &error));
+ g_assert_no_error(error);
+ g_assert_nonnull(istream);
+ doc = poppler_document_new_from_stream(istream, g_file_info_get_size(info), NULL, NULL, &error);
+ g_assert_no_error(error);
+ g_assert_nonnull(doc);
+ g_assert_cmpint(poppler_document_get_n_pages(doc), == , n_pages);
+ g_object_unref(doc);
+ g_object_unref(istream);
+
+ g_object_unref(info);
+ g_object_unref(file);
+#endif
+
return EXIT_SUCCESS;
}
--
2.20.1
>From d3bbf23c8ea09996e75af0d59ec5d786b4cb8677 Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Thu, 14 Feb 2019 09:48:50 +0000
Subject: [PATCH 3/3] Update changelog
---
debian/changelog | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 1525456..028e922 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+poppler (0.71.0-2.1) UNRELEASED; urgency=medium
+
+ * Add patch to fix poppler_document_new_from_stream() regression
+ (Closes: #896596)
+ * d/tests/glib: Add a test for #896596
+
+ -- Simon McVittie <s...@debian.org> Thu, 14 Feb 2019 09:48:34 +0000
+
poppler (0.71.0-2) unstable; urgency=medium
* Upload to unstable.
--
2.20.1
