bugGNU roff - Bugs: bug #64202, [man pages] groff_man(7)...

 
 

bug #64202: [man pages] groff_man(7) inconsistently (and redundantly) guards some .MR references with '\%'

Submitter:  Keith Marshall <keithmarshall>
Submitted:  Tue 16 May 2023 07:20:54 PM UTC
   
 
Category:  General Severity:  3 - Normal
Item Group:  Documentation Status:  Invalid
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 28 Mar 2024 11:32:29 PM UTC, comment #9: 

comment #1:

> 1.  The new `MR` macro unconditionally prefixes its first argument with a `\%` escape sequence to suppress hyphenation.


This is incorrect.  I think I was doing that for a while when I had `MR` under development, but it was not the case when I landed the feature and has not become true subsequently.

> 2.  All of groff's man pages (.[157]) files are produced in the build tree from from .man inputs.


This was and remains true.

> 3.  The reason for that is that man page content is partially dynamic.  The redundancy you're observing is the result of the "@g@" character sequence being replaced by the ./configure-d command prefix in use by the build.  By default, and in many build scenarios (of those I've seen personally, all except for Solaris), this prefix is empty.


This was and remains true.

> 4.  @g@ is expanded in several contexts, not just the first argument of `MR`.  Wherever it occurs, it is part of a literal to which we do not want automatic hyphenation to apply.


This was and remains true.

> 5.  Therefore in some cases you get doubled `\%`s.


This is not true and I have no evidence that it ever was.

(There is one instance of a manually placed[1] doubled `\%` in the sources in Git HEAD.  I've committed a fix to my working copy.  It is not in a man page cross reference or any other sort of link text, so it immaterial to this ticket anyway.)

A built groff 1.23.0 tree does not produce any instances of `\%\%` in any of its man pages, as a grep will readily confirm.
 

> The title mostly says it all.


Your ticket title appears to have been deceptive.  I should have demanded evidence from you at the time; I will have settle for doing so now.

In the absence of such evidence, I regard your claim of bugginess as unfounded.  Changing status to "Invalid".

[1] it was probably a sed(1) script I ran months after the groff 1.23.0 release

G. Branden Robinson <gbranden>
Group administrator
Thu 28 Mar 2024 10:50:28 PM UTC, comment #8: 

comment #7:

> I'm rejecting this.

Fair enough; that is your prerogative.  It doesn't make it any less of a bug; it just makes it a bug which you have decided you will not fix!  In any case, I have a work around.

> If a PDFMark tag indexing system constructs groff identifiers
> from arbitrary input strings, that system is responsible for
> ensuring that the constructed identifiers are syntactically valid.

That is not the issue here.  The issue is that your unnecessary '\%' prefix infiltrates the text of the pdfmark '/Link' annotation, which defines the destination of the resultant PDF hyperlink; there is no attempt to construct a groff identifier from this text.  My work around simply iterates over the initial part of the string, discarding the first character while it is '\%', and keeping the remainder of the string, as soon as its first character becomes anything other than '\%'.

In fact, I am not aware of any code, within pdfmark.tmac, which attempts to construct a groff identifier from arbitrary input text; if you know of any, please do bring it to my attention.

> The groff language has, for decades, supported the `\A` escape
> sequence to assist with this.

I am aware of this.  Indeed, just last week, I did stumble on some code which does attempt to match arbitrary input text to (a part of) a pre-existing groff identifier, using .if d prefix.\\$1 as the sole test.  In the particular context, the effect was completely innocuous, aside from raising an unexpected warning.  The solution, to avoid the warning, was to test .if \A'\\$1' first, and to perform the .if d ... test only if that returns true.

> Unfortunately, it has also lacked a string iterator that would make
> it easier to do something about arbitrary input strings one's tag
> indexing system encounters.  (Apart from aborting the formatter or
> risking incorrect lookups on tag values, that is.)  Providing such
> a facility is contemplated as a future groff language direction
> in bug #62264.

I agree that string iteration could be made more convenient, but it is already possible ... I do it, in my sanitize.tmac, to filter out special codes, which do not render appropriately in PDF outline text.

> I decided to solve this problem in "pdf.tmac" by not constructing
> groff identifiers from arbitrary input strings, instead using
> another string, indexed by the bookmark's sequence number.  This
> does come at the price of turning tag lookup time from constant in
> the number of already-recorded tags to linear.

As I've pointed out above, identifier construction is not relevant to this issue, so what you have done in pdf.tmac has no bearing on what I have had to do, to work around your bug.

Keith Marshall <keithmarshall>
Group Member
Sun 24 Mar 2024 09:27:12 PM UTC, comment #7: 

I'm rejecting this.  If a PDFMark tag indexing system constructs groff identifiers from arbitrary input strings, that system is responsible for ensuring that the constructed identifiers are syntactically valid.

The groff language has, for decades, supported the `\A` escape sequence to assist with this.

Unfortunately, it has also lacked a string iterator that would make it easier to do something about arbitrary input strings one's tag indexing system encounters.  (Apart from aborting the formatter or risking incorrect lookups on tag values, that is.)  Providing such a facility is contemplated as a future groff language direction in bug #62264.

I decided to solve this problem in "pdf.tmac" by not constructing groff identifiers from arbitrary input strings, instead using another string, indexed by the bookmark's sequence number.  This does come at the price of turning tag lookup time from constant in the number of already-recorded tags to linear.

Fortunately, that change appears not to have had significant impact on document rendering time, and costs little even in a document with ~900 bookmark tags.

https://lists.gnu.org/archive/html/groff/2024-03/msg00012.html

G. Branden Robinson <gbranden>
Group administrator
Tue 23 May 2023 03:23:16 PM UTC, comment #6: 

comment #5:

> comment #4:
> > comment #3:
> > ... I don't have enough insight into the implementation you're
> > working on to offer advice.  Maybe you could share some of its
> > code.
>
> I'll post it on OSDN, when I get it to a "commit-ready" state.
> I do have it working, quite nicely, now, but with my MR macro
> override "uglified" by the need to filter out '\%' prefixes,
> from its first argument.

You can find it at groff-pdfmark commit #918f8bd -- look, in particular, for the new tmac/anpdf.tmac file.

Keith Marshall <keithmarshall>
Group Member
Mon 22 May 2023 10:14:50 AM UTC, comment #5: 

comment #4:

> comment #3:
> > comment #1:
> > > Hi Keith,
> > >
> > > I'm aware of this.  It's deliberate insofar as it's a consequence of other decisions.
> > >
> > > The main facts are these:
> > >
> > > 1.  The new `MR` macro unconditionally prefixes its first argument with a `\%` escape sequence to suppress hyphenation.
> >
> > That's what I thought.  Consequently, there is absolutely no need for references, such as '.MR \%topic n', to ever add that redundant '\%' prefix to the topic name.
>
> ...and there are no cases of it doing so in the groff tree,

Seriously?  Is your day job related to government, in some shape or form?  You certainly seem to exhibit the politician's proclivity to spew verbiage, with little. or no, bearing on the issue at hand.

> $ git grep 'MR.*\\%' || echo NONE
> NONE

A badly designed experiment, testing a bogus hypothesis, might be viewed as disingenuous obfuscation ... perhaps not so in this case, but rather a misunderstanding of the issue?  In reality, there are 197 cases arising from abuse of '.MR @g@...', and one further, from abuse of '.MR @PSPRINT@ ...'

$ hg grep '^\. *MR  *@g@' | wc -l
197

$ hg grep '^\. *MR  *@' | wc -l
198

$ hg grep '^\. *MR  *@[^g]'
src/roff/groff/groff.1.man:.MR @PSPRINT@ 1 .


> [...irrelevant verbiage snipped...]


> The purpose is not being subverted.

I guess we'll just have to agree to disagree, on that conclusion.

> You said yourself that the "seat" of this behavior is Makefile rules for generating .[157] from .man.  It would be wrong to do so in "makevarescape.sed", for instance, because '@g@' and friends get expanded in contexts other than roff sources.  Moreover, valid roff input is indeed being produced.
>
> [...more verbiage snipped...]


> This is why I mentioned the following point in comment #1.
>
> > You do not need to sanitize content destined for device control escape sequences (or the `device` request) of the `\%` escape sequence.  The formatter will ignore this escape sequence in that context, skipping over it without diagnostic, and it will not appear in the "x X" commands that GNU troff produces.  This is already the case in groff 1.22.4 and therefore I suspect it's been true for many years.


Well, it does propagate through pdfmark output to grops.  Perhaps the pdfmark macro should sanitize its output, but that seems like a huge overhead in tmac code ... every single byte of pdfmark throughput would need to be inspected, just to weed out a miniscule few rogues.

> Are you wrapping or replacing the `MR` macro

Yes, and ...

> and "sanitizing" its first argument for some other purpose?

...no; it's a consequence of not sanitizing the first argument, coupled with your abuse of '@g@', (and maybe also of '@PSPRINT@'), in man page source, that allows the unwanted '\%' to propagate.

> You said:
>
> > (which, in its present state of development, does not incur any address sanitizer overhead)
>
> ...which I didn't completely understand, as ASAN doesn't seem relevant to the present discussion of roff macro processing.
>
> Leaving in "Need Info" status, as I'm stuck; I don't agree with your implication that repeated leading \% escape sequences in a word are invalid roff input,

I neither said, nor even remotely implied, any such thing.

> and I don't have enough insight into the implementation you're working on to offer advice.  Maybe you could share some of its code.

I'll post it on OSDN, when I get it to a "commit-ready" state.  I do have it working, quite nicely, now, but with my MR macro override "uglified" by the need to filter out '\%' prefixes, from its first argument.

Keith Marshall <keithmarshall>
Group Member
Wed 17 May 2023 07:38:54 PM UTC, comment #4: 

comment #3:

> comment #1:
> > Hi Keith,
> >
> > I'm aware of this.  It's deliberate insofar as it's a consequence of other decisions.
> >
> > The main facts are these:
> >
> > 1.  The new `MR` macro unconditionally prefixes its first argument with a `\%` escape sequence to suppress hyphenation.
>
> That's what I thought.  Consequently, there is absolutely no need for references, such as '.MR \%topic n', to ever add that redundant '\%' prefix to the topic name.


...and there are no cases of it doing so in the groff tree,


$ git grep 'MR.*\\%' || echo NONE
NONE


so your stridency here is a bit puzzling.
 

> > 2.  All of groff's man pages (.[157]) files are produced in the build tree from from .man inputs.
>
> Again, I'm well aware of this, but the '*.man' sources do not specify the redundant prefix,


Agreed.

> (other than incidentally, via a malformed transform for a '@g@' prefix).


That's not incidental, it's deliberate.
 

> And, therein lies the bug ... for it is a bug.  The intent of '@g@' is to add a program name prefix -- typically 'g' for GNU programs -- so that 'tbl' becomes 'gtbl', when appropriate; it has absolutely no business to ever include '\%' as part of that prefix.


Why not?  According to DWB, Heirloom Doctools, and GNU troffs, it's idempotent when repeated at the beginning of a word.


$ cat EXPERIMENTS/hyphenation-point.roff
.ll 3n
foo
\%foo
\%\%foo
\%\%\%foo
A \%\%\%foo
AB \%\%\%foo
ABC \%\%\%foo
ABCD \%\%\%foo
.pl \n(plu
$ nroff -Wbreak EXPERIMENTS/hyphenation-point.roff
foo
foo
foo
foo
A
foo
AB
foo
ABC
foo
ABCD
foo


(I suppressed warnings because they're not relevant here; only spurious hyphens at the start of a word would be, and those would be visible in the output anyway.  I also tried all three formatters with line lengths of 4n and 5n; this also failed to cause spurious hyphenation.)

Formatters are prepared to handle inputs like this, and so too should macro packages be, if they want to claim general utility.

> (FWIW, the seat of the bug is within the substitution for '@g@', as it is specified in the generated Makefile, at the point where 'topic.n' is generated from 'topic.n.man').


It's done for some other replacements as well.


commit d84d9e1d85287b24d14001a6fdcbaa9cfc588d55
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Sun Feb 20 05:21:36 2022 +1100

    Makefile.am: Use hyphenation control escapes more.

    * Makefile.am (.man): Prefix hyphenation control escape sequences to
      more configuration-time interpolations to prevent their hyphenation:
      @DEVICE@, @g@, @INDEX_SUFFIX@, @PAGE@, @TMAC_{AN,M,S}_PREFIX@,
      @TMAC_MDIR@.


(That commit message is a little unfortunate.  It should say "configuration-dependent", not "configuration-time".)

> Understood.  However, the intent of '@g@' should not be subverted, for this unrelated purpose ... either specify '\%' explicitly, in any context where it is intended, or introduce a specific transform, other than '@g@' itself, which implies the effect of '\%@g@'.


The purpose is not being subverted.  You said yourself that the "seat" of this behavior is Makefile rules for generating .[157] from .man.  It would be wrong to do so in "makevarescape.sed", for instance, because '@g@' and friends get expanded in contexts other than roff sources.  Moreover, valid roff input is indeed being produced.

> I think that this is an insidious bug, which should be fixed.


I checked out your attached PDF and it looks quite nice to me.  The problem with the hyperlinks is clear, and as you described; a stray percent sign is getting into some of the hyperlink targets you generate.  This is not the fault of the formatter or the man page sources.  If it were, then the hyperlinks that groff Git produces would have the same problem.


$ ./build/test-groff -t -rU1 -man -Tutf8 -Z ./build/tmac/groff_man.7 | grep 'x X' | tail -n 20
x X devtag:.NH 1
x X devtag:.eo.h
x X tty: link man:tbl(1)
x X tty: link
x X tty: link man:eqn(1)
x X tty: link
x X tty: link man:refer(1)
x X tty: link
x X tty: link man:man(1)
x X tty: link
x X tty: link man:groff_mdoc(7)
x X tty: link
x X tty: link man:groff_man_style(7)
x X tty: link
x X tty: link man:groff(7)
x X tty: link
x X tty: link man:groff_char(7)
x X tty: link
x X tty: link man:man(7)
x X tty: link
$ ./build/test-groff -t -man -Thtml -Z ./build/tmac/groff_man.7 | grep 'x X' | tail -n 25
x X devtag:.br
x X html:<a href="man:tbl(1)">
x X html:</a>
x X html:<a href="man:eqn(1)">
x X html:</a>
x X html:<a href="man:refer(1)">
x X html:</a>
x X devtag:.sp 1
x X devtag:.br
x X html:<a href="man:man(1)">
x X html:</a>
x X devtag:.sp 1
x X devtag:.br
x X html:<a href="man:groff_mdoc(7)">
x X html:</a>
x X devtag:.sp 1
x X devtag:.br
x X html:<a href="man:groff_man_style(7)">
x X html:</a>
x X html:<a href="man:groff(7)">
x X html:</a>
x X html:<a href="man:groff_char(7)">
x X html:</a>
x X html:<a href="man:man(7)">
x X html:</a>


This is why I mentioned the following point in comment #1.

> You do not need to sanitize content destined for device control escape sequences (or the `device` request) of the `\%` escape sequence.  The formatter will ignore this escape sequence in that context, skipping over it without diagnostic, and it will not appear in the "x X" commands that GNU troff produces.  This is already the case in groff 1.22.4 and therefore I suspect it's been true for many years.


Are you wrapping or replacing the `MR` macro and "sanitizing" its first argument for some other purpose?  You said:

> (which, in its present state of development, does not incur any address sanitizer overhead)


...which I didn't completely understand, as ASAN doesn't seem relevant to the present discussion of roff macro processing.

Leaving in "Need Info" status, as I'm stuck; I don't agree with your implication that repeated leading \% escape sequences in a word are invalid roff input, and I don't have enough insight into the implementation you're working on to offer advice.  Maybe you could share some of its code.

G. Branden Robinson <gbranden>
Group administrator
Wed 17 May 2023 03:10:58 PM UTC, comment #3: 

comment #1:

> Hi Keith,
>
> I'm aware of this.  It's deliberate insofar as it's a consequence of other decisions.
>
> The main facts are these:
>
> 1.  The new `MR` macro unconditionally prefixes its first argument with a `\%` escape sequence to suppress hyphenation.


That's what I thought.  Consequently, there is absolutely no need for references, such as '.MR \%topic n', to ever add that redundant '\%' prefix to the topic name.

> 2.  All of groff's man pages (.[157]) files are produced in the build tree from from .man inputs.


Again, I'm well aware of this, but the '*.man' sources do not specify the redundant prefix, (other than incidentally, via a malformed transform for a '@g@' prefix).

> 3.  The reason for that is that man page content is partially dynamic.  The redundancy you're observing is the result of the "@g@" character sequence being replaced by the ./configure-d command prefix in use by the build.  By default, and in many build scenarios (of those I've seen personally, all except for Solaris), this prefix is empty.


And, therein lies the bug ... for it is a bug.  The intent of '@g@' is to add a program name prefix -- typically 'g' for GNU programs -- so that 'tbl' becomes 'gtbl', when appropriate; it has absolutely no business to ever include '\%' as part of that prefix.

(FWIW, the seat of the bug is within the substitution for '@g@', as it is specified in the generated Makefile, at the point where 'topic.n' is generated from 'topic.n.man').

> 4.  @g@ is expanded in several contexts, not just the first argument of `MR`.  Wherever it occurs, it is part of a literal to which we do not want automatic hyphenation to apply.


Understood.  However, the intent of '@g@' should not be subverted, for this unrelated purpose ... either specify '\%' explicitly, in any context where it is intended, or introduce a specific transform, other than '@g@' itself, which implies the effect of '\%@g@'.

> ...snip...
>
> What do you think?


I think that this is an insidious bug, which should be fixed.

Keith Marshall <keithmarshall>
Group Member
Wed 17 May 2023 03:38:56 AM UTC, comment #2: 

I forgot to run that demonstration with all warnings enabled.

The result does not change.


$ printf '\\X@ps: foo\\%%bar@\n' | troff -ww | grep 'x X'
x X ps: foobar
$ printf '\\X@ps: foo\\%%bar@\n' | troff -ww -z 2>&1 | grep . || echo NO ERROR
NO ERROR


G. Branden Robinson <gbranden>
Group administrator
Wed 17 May 2023 03:35:52 AM UTC, comment #1: 

Hi Keith,

I'm aware of this.  It's deliberate insofar as it's a consequence of other decisions.

The main facts are these:

1.  The new `MR` macro unconditionally prefixes its first argument with a `\%` escape sequence to suppress hyphenation.

2.  All of groff's man pages (.[157]) files are produced in the build tree from from .man inputs.

3.  The reason for that is that man page content is partially dynamic.  The redundancy you're observing is the result of the "@g@" character sequence being replaced by the ./configure-d command prefix in use by the build.  By default, and in many build scenarios (of those I've seen personally, all except for Solaris), this prefix is empty.

4.  @g@ is expanded in several contexts, not just the first argument of `MR`.  Wherever it occurs, it is part of a literal to which we do not want automatic hyphenation to apply.

5.  Therefore in some cases you get doubled `\%`s.

In the short term (that is, groff 1.23.0 final), I don't think there is anything that can be done to correct this without frustrating the objective in #4 above, or without increasing the number of magic character sequences that get replaced in man pages ("@g@" and "@gmr@"?).  I'm not sanguine about the latter as a solution because I think it will increase the man page maintenance burden.  People will forget or fail to learn which sequence to use, and use the wrong one.  (I include myself in that set, because when confronted with complex systems, I forget things.)

> specifying such a guard in the macro call imposes an unnecessary burden on any macro implementation which constructs, for example, a PDF external reference based on the MR macro arguments, because the generated reference address must be passed through a sanitizer routine -- an unnecessary overhead, when MR already prepends the '\%' in running text.


There are two things to note with respect to the above challenge.

A.  You do not need to sanitize content destined for device control escape sequences (or the `device` request) of the `\%` escape sequence.  The formatter will ignore this escape sequence in that context, skipping over it without diagnostic, and it will not appear in the "x X" commands that GNU troff produces.  This is already the case in groff 1.22.4 and therefore I suspect it's been true for many years.

$ printf '\\X@ps: foo\\%%bar@\n' | troff | grep 'x X'
x X ps: foobar

B.  I've noticed that operations like sanitizing a string (/macro/diversion), or iterating over one for any reason, is hell on wheels.  (I got my taste of this when implementing ellipsis-ification of man page headers and footers to resolve bug #43532 and bug #62257.)

Therefore I have proposed (and I've mentioned this on the groff list, to no response so far), adding a request to do exactly this: iterate over strings (macros, diversions).  To do this right, especially for diversions, will likely require additional conditional-expression predicates, because an item in a diversion can be something that does not exist in the formatter's input language (namely, a "node", but I do not yet know if there are other such ineffable data types one can encounter--I expect to find out upon attempting an implementation).

See bug #62264.

What do you think?

G. Branden Robinson <gbranden>
Group administrator
Tue 16 May 2023 07:20:54 PM UTC, original submission:  

The title mostly says it all.  When groff_man(7) refers to tbl(1), eqn(1), and refer(1), it does so using a reference of the form:

.MR \%tbl 1

while, when referring to any other page, such as groff(1), it omits the '\%' hyphenation guard prefix:

.MR groff 1

In this context, the '\%' prefix is redundant; the MR macro -- at least in the fall back within the man page source -- supplies an additional one anyway.  Furthermore, specifying such a guard in the macro call imposes an unnecessary burden on any macro implementation which constructs, for example, a PDF external reference based on the MR macro arguments, because the generated reference address must be passed through a sanitizer routine -- an unnecessary overhead, when MR already prepends the '\%' in running text.

To illustrate the issue, I've attached a copy of groff_man.7.pdf, generated from your git master source, and formatted by my development copy of pdfroff, with complementary '-manpdf', (which, in its present state of development, does not incur any address sanitizer overhead):

GROFF_TMAC_PATH=/path/to/groff-pdfmark/tmac ./pdfroff -manpdf groff_man.7 > groff_man.7.pdf

Note the malformed https references for tbl(1), eqn(1), and refer(1), in the "See also" section,for example.

Keith Marshall <keithmarshall>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #54755:  groff_man.7.pdf added by keithmarshall (71KiB - application/pdf - Illustration of malformed reference link addresses)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by keithmarshall (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    23:32 gbranden StatusRejected Invalid
    2024-03-24 gbranden StatusNeed Info Rejected
        Open/ClosedOpen Closed
    2023-05-17 gbranden Summary[man-pages]: groff_man(7) inconsistently (and redundantly) guards some .MR references with '\%' [man pages] groff_man(7) inconsistently (and redundantly) guards some .MR references with '\%'
    2023-05-17 gbranden CategoryMacro man General
        Item GroupNone Documentation
        StatusNone Need Info
        Assigned toNone gbranden
    2023-05-16 keithmarshall Attached File- Added groff_man.7.pdf, #54755

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code