Cannot save modified document on Android

Bug #2000871 reported by ihtarlik
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
qpdfview
Fix Released
Low
Adam Reichold

Bug Description

Running Debian in proot environment on Termux for Android 11 (aarch64). I open a PDF form which allows for editing, make changes, then attempt to save them. I get an error box labeled "Warning" which says "Could not refresh '/home/ihtarlik/form_2241.pdf'." When I attempt to "Save as", I get a similar error saying the file I am attempting to write cannot be opened. Disturbingly, when I close qpdfview, it zeroes the file I originally opened (it keeps its name, but reports being 0 bytes in size).

When run from the terminal, I get the following output:

qt.qpa.xcb: XKeyboard extension not present on the X server
QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '/tmp/runtime-ihtarlik'
Unknown MIME type: "application/x-zerosize"
"Could not match file type of 'form_2241.pdf'!"

I have checked permissions (trying various modes including 777) and running as root, but getting the same error. Editing the same file on an Ubuntu 22.04 VPS produces no problems.

Revision history for this message
Adam Reichold (adamreichold) wrote :

Hello,

PDF documents are not modified in place but rather the new version that includes that changes existing only in memory are written out to a temporary file. After that, the original file is overwritten (so opened for writing and truncated) with the contents of that temporary file. (This applies both to "Save" as well as "Save as", only the target paths differ.)

The failure mode you describe suggests that either writing out data to the temporary file fails silently or that reading the data back from the temporary file is unsuccessful. Since both explanations would require that errors not reported by the file system or ignored within qpdfview and or Poppler (the underlying PDF engine), I fear that this could only be resolved by stracing the qpdfview process to figure out which system calls yield errors.

Regards,
Adam

summary: - Warning: Could not refresh {filename}
+ Cannot save modified document on Android
Revision history for this message
ihtarlik (ihtarlik) wrote :

I ran the program with strace and have attached the logged output.

Revision history for this message
Adam Reichold (adamreichold) wrote (last edit ):
Download full text (5.2 KiB)

Hello again,

the salient part of the log is

newfstatat(AT_FDCWD, "/tmp", {st_mode=S_IFDIR|0700, st_size=4096, ...}, AT_SYMLINK_NOFOLLOW) = 0
openat(AT_FDCWD, "/tmp", O_RDWR|O_CLOEXEC|O_TMPFILE, 0600) = 26
lseek(26, 0, SEEK_SET) = 0
getrandom("\x42\x75\x62\x49\xd0\x70\x54\x66\x1c\x97\xee\x3d\x9c\xbb\x7c\x95\x9e\x5e\x58\x33\x03\xf2\x50\x4b\x58\x12\xb3\xd0\xa3\xab\xb7\x2d"..., 256, 0) = 256
getrandom("\x4e\x80\xff\xd5\x64\x88\xb6\x26\x00\x59\xa3\x7c\xea\x3c\xa6\xcd\x14\xc3\xae\xe3\x10\x14\x62\xa1\x46\xbe\xf3\x27\x38\x13\x28\x28"..., 256, 0) = 256
getrandom("\xc3\x75\x28\x54\xde\xc9\xe0\xfd\x3a\xe3\xd0\x65\x2f\x3d\x4a\xf1\xdb\x2e\xe6\x3d\x74\x96\xc4\x30\x8a\x44\x6d\xe8\x61\x6f\x05\x5c"..., 256, 0) = 256
getrandom("\x51\x93\xb0\xa6\x69\xba\x10\x57\x60\xff\xc9\xc8\xb1\x18\x52\x67\x2d\x7e\x42\x29\xf9\xb4\x58\x74\xda\x84\xc4\x2a\xf0\x6d\x2b\x35"..., 256, 0) = 256
getrandom("\x4f\xbc\x78\x61\xa3\x21\xf1\xe0\x65\x32\x1a\x00\x37\x59\xd1\xe0\xf4\x6f\xf9\xae\x40\x5e\x6b\x49\xf5\xbf\x16\x2b\x5d\xfa\xf9\xfa"..., 256, 0) = 256
getrandom("\xb7\xd4\xa5\x73\xe2\x9f\x69\x53\x1f\x72\x43\x1b\x84\xa2\xd3\x47\xa3\xdd\x95\x00\x8d\xd1\x2d\x85\x27\x55\x5a\x39\xbb\x29\x45\x5a"..., 256, 0) = 256
getrandom("\x84\x4d\xcd\x68\x95\x42\x79\x1e\xd7\x0f\x75\xc3\xae\xcb\x85\xcc\xc0\xe8\xb4\xf4\x0b\xf4\x46\x7a\x94\xa7\x79\xc6\xd8\x3f\xc3\x79"..., 256, 0) = 256
getrandom("\x8f\x37\x67\x87\x35\x28\x0c\x3e\xda\x3d\xae\xa0\xd5\x2e\xb3\xa3\x26\x25\x92\x7d\x79\x9a\x01\xfa\x01\x14\xac\x1b\x63\x6f\x8f\x46"..., 256, 0) = 256
getrandom("\x07\xbe\xa7\xe6\x7d\x28\xe1\x82\x2a\x48\x56\xd4\x5b\xfc\x8f\x4d\x1c\x31\x9f\xac\xe0\x9c\x5c\x55\x39\xd0\x26\xe5\x7a\x7a\xca\xa9"..., 256, 0) = 256
getrandom("\x85\x54\xc8\x17\xff\x51\x74\x7c\x99\x82\x5c\x00\x1c\xda\xd3\x24\xee\x64\xb8\x4a\xa2\x37\x6c\xa0\x08\x1f\x36\x87\xa2\xdb\x4b\x06"..., 192, 0) = 192
linkat(AT_FDCWD, "/proc/self/fd/26", AT_FDCWD, "/tmp/qpdfview.zgSJJG.pdf", AT_SYMLINK_FOLLOW) = 0
openat(AT_FDCWD, "/tmp/qpdfview.zgSJJG.pdf", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 27
statx(27, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL, stx_attributes=STATX_ATTR_ENCRYPTED, stx_mode=S_IFREG|0600, stx_size=0, ...}) = 0
statx(AT_FDCWD, "/tmp/qpdfview.zgSJJG.pdf", AT_STATX_SYNC_AS_STAT|AT_SYMLINK_NOFOLLOW, STATX_ALL, {stx_mask=STATX_ALL, stx_attributes=STATX_ATTR_ENCRYPTED, stx_mode=S_IFREG|0600, stx_size=0, ...}) = 0
fstat(23, {st_mode=S_IFREG|0600, st_size=822188, ...}) = 0
pread64(23, "%PDF-1.6\r%\342\343\317\323\r\n510 0 obj\r<</Lin"..., 256, 0) = 256
pread64(23, "0>]/Index[510 20]/Info 509 0 R/L"..., 256, 256) = 256

[...]

pread64(23, "\r\nendstream\rendobj\r171 0 obj\r<</"..., 256, 821576) = 256
pread64(23, "h\336L\217Aj\3030\20E\2572\273H\213\310#\331J\342\22\2\301^tS0\270n6\331("..., 208, 821368) = 208
write(27, " "..., 3649) = 3649
close(27) = 0
openat(AT_FDCWD, "2241.pdf", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 27
statx(26, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL, stx_attributes=STATX_ATTR_ENCRYPTED, stx_mode=S_IFREG|0600, stx_size=0, ...}) = 0
read(26, "", 16384) = 0
statx(27, "...

Read more...

Revision history for this message
Adam Reichold (adamreichold) wrote :

This would be a minimal patch which explicitly rewinds instead of closing-then-opening the temporary file.

Revision history for this message
ihtarlik (ihtarlik) wrote :

I am cloning the code branch in bzr and will (attempt to) apply the patch imminently. However, i am wondering if maybe the problem isn't unique to proot environments being run with privileged access to Android's internals like /proc subsystem. Just thinking out loud.

Revision history for this message
Adam Reichold (adamreichold) wrote :

> However, i am wondering if maybe the problem isn't unique to proot environments being run with privileged access to Android's internals like /proc subsystem. Just thinking out loud.

Yes, I think that is very much possible. But it also doesn't mean that we should not try to find a way in which qpdfview works in this scenario. For example, we also carry code so that it works on AmigaOS and OS/2 which could probably be considered equally or even more exotic.

Revision history for this message
ihtarlik (ihtarlik) wrote :

I ran the prepped tarball of 2146 and it generated the same error. I have attached the latest strace log.

Revision history for this message
Adam Reichold (adamreichold) wrote :

So I guess this related to the proot environment after all.

I think it would be good to choose a different idiom for overwriting the file. Instead of using a temporary file managed by the operating system, we could try to create a file next to target file, i.e. if /foo/bar.pdf is modified, we create a temporary file at /foo/bar.pdf.part, then copy over to /foo/bar.pdf and finally delete /foo/bar.pdf.part. The reasoning being that we are not atomic anyway since we overwrite the contents of the target file (to keep all its permissions, extended attributes, etc.) and if we are able to read/write the target, we are more likely to read/write a regular file next to it.

Not sure when I will find the time to implement this though...

Revision history for this message
ihtarlik (ihtarlik) wrote :

Well, at least you know what the issue is likely to be when someone mentions proot. Thanks for your help! I will keep using qpdfview on non-proot platforms until this bug is resolved.

Changed in qpdfview:
status: New → Confirmed
importance: Undecided → Low
Revision history for this message
Adam Reichold (adamreichold) wrote :

Hello again,

I just pushed trunk revision 2147 which implements a simpler fix than outlined above: Instead of continuing to use the O_TMPFILE file descriptor, but writing to and reading from the temporary file is done via its file path. I hope that this resolves whatever inconsistency leads to qpdfview being unable to read back the contents of the temporary file in proot environments.

Could you give this another try? Thanks!

Regards,
Adam

Revision history for this message
ihtarlik (ihtarlik) wrote :

It worked flawlessly! I made changes, saved the document, and reopened it to test the changes were made. Everything looks good now. Thanks!

Revision history for this message
Adam Reichold (adamreichold) wrote :

Glad that it was possible to resolve this.

Changed in qpdfview:
status: Confirmed → Fix Committed
assignee: nobody → Adam Reichold (adamreichold)
milestone: none → 0.5.0
Changed in qpdfview:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.