Error messages from less in version 1.3.2

[praxanz]

With the environment variable PAGER unset, running csview with a filename, results in less showing the error message:

There is no -<E8> option ("less --help" for help)
Press RETURN to continue

when using standard input, the error is:

There is no -l option ("less --help" for help)
Press RETURN to continue

Running these commands with ltrace shows that when the string LESS=-SF is passed to the function putenv(), the trailing null character is missing and extra garbage characters are added to the environment variable:

putenv("LESS=-SF\350\331\002\354p") or putenv("LESS=-SFless")

Not being a rust programmer, I don't know the best way to correct this, but as a workaround, either one of the quick and dirty patches below fixes the issue for me.

In main.rs replace the line

Err(_) => Pager::with_pager("less").pager_envs(["LESS=-SF"]).setup(), 

with either

Err(_) => Pager::with_pager("less").pager_envs(["LESS=-SF\0"]).setup(),

or else with

Err(_) => Pager::with_pager("less -SF").setup(),

testing environment

TERMUX_APK_RELEASE=F_DROID
TERMUX_VERSION=0.118.0
Packages CPU architecture: aarch64
Android version: 14
rustc 1.78.0 (9b00956e5 2024-04-29) (built from a source tarball)
csview 1.3.2
~/.cargo/registry/src/index.crates.io-.../pager-0.16.1/

[rvstaveren]

Same issues seen on FreeBSD 14.1-RELEASE with rust 1.77,

Using Err(_) => Pager::with_pager("less -SF").setup() mitigates the issue for me

[praxanz]

This is a bug in pager-0.16.1

The function putenv() is called with an OsString, where a CString should be used.

There is a merge request pending since last year to correct this.

https://gitlab.com/imp/pager-rs/-/merge_requests/8

At this moment the use of pager_envs() with strings that don't end with a "...\0" can cause undefined results.

[wfxr]

Hi @praxanz, apologize for the delayed response as I've been swamped with work lately. And thank you for your efforts in investigating this issue. Also thanks @rvstaveren for the feedback.

I think the first workaround is better, since it prevents the pager's behavior from being affected by existing environment variables.

@praxanz Would you be willing to submit a pull request? Or I implement it as you suggested.

[praxanz]

I am still trying to learn how to work with git and I don't know yet how to submit a pull request. So, please go ahead and implement one of my proposed workarounds (I prefer the one with the null terminated string). The best long term solution is that you try to contact the authors of pager-0.16.1 and insist that they correct their code, as this bug will also affect other users of pager. Once pager has been corrected, the workaround should be removed from csview.

[wfxr]

@praxanz Got it. I will fix it soon. Would you mind telling me your email so I can add you as a co-author? Although this commit may seem like a one-line change, your contribution should be noted.

[praxanz]

No need to add me as a co-author for this two character temporary fix that will be removed as soon as the upstream bug is corrected. My only contribution was discovering this bug and doing some diagnostic work.

Small remark: In the comment in your pull request you mention merge request #8 in the upstream code base, but there is also a different merge request #10 that tries to solve the same problem.