Bug: mailing list linkification in headers incorrect

Bug: mailing list linkification in headers incorrect

[Konstantin Ryabitsev]

Hello:

Small thing I noticed today while poking around is that the links around
mailing lists in the headers don't actually go to the right place. E.g.:

https://lore.kernel.org/bpf/20240909201652.319406-1-mathieu.desnoyers@efficios.com/

The link around linux-kernel@vger.kernel.org goes to
https://lore.kernel.org/bpf/20240909201652.319406-1-mathieu.desnoyers@efficios.com/lkml/?t=20240909201716,
though it is probably intended to point at /lkml/?t=...

-K

Re: Bug: mailing list linkification in headers incorrect

[Filip Hejsek]

Hi,


> Small thing I noticed today while poking around is that the links around
> mailing lists in the headers don't actually go to the right place. E.g.:
> 
> [...]

this is because the links use a relative URL:

	Cc: <a
	href="lkml/?t=20240909201716">linux-kernel@vger.kernel.org</a>
,

This URL is constructed by Inbox::base_url, which in this case uses a
relative URL from your config.

(BTW this is the issue I originally wanted to report before I
procrastinated by investigating the addr2url mapping corruption)

 - Filip

Re: Bug: mailing list linkification in headers incorrect

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> Hello:
> 
> Small thing I noticed today while poking around is that the links around
> mailing lists in the headers don't actually go to the right place. E.g.:
> 
> https://lore.kernel.org/bpf/20240909201652.319406-1-mathieu.desnoyers@efficios.com/
> 
> The link around linux-kernel@vger.kernel.org goes to
> https://lore.kernel.org/bpf/20240909201652.319406-1-mathieu.desnoyers@efficios.com/lkml/?t=20240909201716,
> though it is probably intended to point at /lkml/?t=...

I'm working on a fix, but using publicinbox.nameIsUrl=true and
getting rid of all the publicinbox.*.url entries avoids the
problem (and saves some memory)

Re: Bug: mailing list linkification in headers incorrect

[Konstantin Ryabitsev]

On Mon, Sep 09, 2024 at 10:11:28PM GMT, Eric Wong wrote:
> I'm working on a fix, but using publicinbox.nameIsUrl=true and
> getting rid of all the publicinbox.*.url entries avoids the
> problem (and saves some memory)

I'm happy to set that, but the reason url= entries exist at all is because
public-inbox-init requires a HTTP_URL positional argument. Maybe it can be
changed to be "-c url=whatnot" instead of positional?

-K

Re: [PATCH] view: fix x-post links for relative urls

[Konstantin Ryabitsev]

On Tue, Sep 10, 2024 at 12:40:48AM GMT, Eric Wong wrote:
> We need to make correct relative URL paths for users configuring
> publicinbox.$NAME.url as relative URL paths (e.g. matching the
> inbox `$NAME').
> 
> Users of protocol-relative (e.g. `//$HOST/$NAME') and absolute URIs
> (e.g `https://example.com/$NAME') were unaffected by this bug.
> Users relying on publicinbox.nameIsUrl and omitting
> publicinbox.*.url entries were also immune to this bug.

I've applied the patch, but I'm not sure it's quite what was intended as none
of the lists are linkified any more.

-K

Re: [PATCH] view: fix x-post links for relative urls

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Tue, Sep 10, 2024 at 12:40:48AM GMT, Eric Wong wrote:
> > We need to make correct relative URL paths for users configuring
> > publicinbox.$NAME.url as relative URL paths (e.g. matching the
> > inbox `$NAME').
> > 
> > Users of protocol-relative (e.g. `//$HOST/$NAME') and absolute URIs
> > (e.g `https://example.com/$NAME') were unaffected by this bug.
> > Users relying on publicinbox.nameIsUrl and omitting
> > publicinbox.*.url entries were also immune to this bug.
> 
> I've applied the patch, but I'm not sure it's quite what was intended as none
> of the lists are linkified any more.

That shouldn't happen, and I can't seem to reproduce it,
either...

You're still using "url = $NAME" as posted in
https://public-inbox.org/meta/20240903-woodoo-airborne-harrier-6733c5@meerkat/
, right?

IOW this:

	[publicinbox "tools"]
		url = tools
		...

Re: [PATCH] view: fix x-post links for relative urls

[Konstantin Ryabitsev]

On Tue, Sep 10, 2024 at 08:25:17PM GMT, Eric Wong wrote:
> That shouldn't happen, and I can't seem to reproduce it,
> either...
> 
> You're still using "url = $NAME" as posted in
> https://public-inbox.org/meta/20240903-woodoo-airborne-harrier-6733c5@meerkat/
> , right?
> 
> IOW this:
> 
> 	[publicinbox "tools"]
> 		url = tools
> 		...

That's correct, the configs are exactly as in that message, with list details
being the only variation per each defined list.

-K

[PATCH] www: preload all inboxes if using ->ALL

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Tue, Sep 10, 2024 at 08:25:17PM GMT, Eric Wong wrote:
> > That shouldn't happen, and I can't seem to reproduce it,
> > either...

Oh duh, I'm pretty sure the below fixes.  Losing my mind from
heat and ant infestations :<

-------------8<-----------
Subject: [PATCH] www: preload all inboxes if using ->ALL

This ought to improve memory layout and ensure the regexp
for address => inbox linkification works when hitting
/$EXTINBOX/$MSGID/ links first (instead of /$INBOX/$MSGID)

This fill_all call is redundant for cindex users who get the
preload anyways, but necessary for non-cindex users.

This should also avoid the broken/empty regexps problem described in
3b51fcc196e3 (view: fix addr2url mapping corruption, 2024-09-06)

Fixes: 48cbe0c3c8dc4d26 (www: linkify inbox addresses in To/Cc headers, 2024-01-09)
Reported-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Link: https://public-inbox.org/meta/20240910-strong-rainbow-warthog-d6fe69@lemur/
---
 lib/PublicInbox/WWW.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 289599b8..1bc2966d 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -187,7 +187,10 @@ sub preload {
 		if (defined($pi_cfg->{'publicinbox.cgitrc'})) {
 			$pi_cfg->limiter('-cgit');
 		}
-		$pi_cfg->ALL and require PublicInbox::Isearch;
+		if ($pi_cfg->ALL) {
+			require PublicInbox::Isearch;
+			$pi_cfg->fill_all;
+		}
 		$self->cgit;
 		$self->coderepo;
 		$self->stylesheets_prepare($_) for ('', '../', '../../');

Re: [PATCH] www: preload all inboxes if using ->ALL

[Konstantin Ryabitsev]

On Wed, Sep 11, 2024 at 09:25:49PM GMT, Eric Wong wrote:
> Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> > On Tue, Sep 10, 2024 at 08:25:17PM GMT, Eric Wong wrote:
> > > That shouldn't happen, and I can't seem to reproduce it,
> > > either...
> 
> Oh duh, I'm pretty sure the below fixes.

It does! Looks great now.

Tested-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>

> Losing my mind from heat and ant infestations :<

Sorry to hear that -- hopefully things get a bit better in the fall?

-K

Re: [PATCH] www: preload all inboxes if using ->ALL

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On Wed, Sep 11, 2024 at 09:25:49PM GMT, Eric Wong wrote:
> > Oh duh, I'm pretty sure the below fixes.
> 
> It does! Looks great now.
> 
> Tested-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>

Thanks, applied as commit 9ab83286f8cff42fb3657dda2cd8a86fb068a59c

> > Losing my mind from heat and ant infestations :<
> 
> Sorry to hear that -- hopefully things get a bit better in the fall?

*shrug* probably a last gasp of summer heatwave, but often
September is hottest.  But it's been miserable all summer :<