user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
* [PATCH] Allow specification of the number of search results to return
@ 2018-03-26 22:34 Jonathan Corbet
  2018-03-27 10:44 ` Eric Wong
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Corbet @ 2018-03-26 22:34 UTC (permalink / raw)
  To: meta

Add an "l=" parameter to the search query syntax to specify how many
results should be returned.  The default remains 200.
---
200 is a lot of results for one page, so allow it to be tweaked.  I've not
added anything to the query form to access this (don't need it) but could.

 lib/PublicInbox/SearchView.pm | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm
index 53b88c3..1ac72b2 100644
--- a/lib/PublicInbox/SearchView.pm
+++ b/lib/PublicInbox/SearchView.pm
@@ -35,7 +35,7 @@ sub sres_top_html {
 	my $code = 200;
 	# double the limit for expanded views:
 	my $opts = {
-		limit => $LIM,
+		limit => $q->{l},
 		offset => $q->{o},
 		mset => 1,
 		relevance => $q->{r},
@@ -182,6 +182,7 @@ sub search_nav_bot {
 	my $total = $mset->get_matches_estimated;
 	my $nr = scalar $mset->items;
 	my $o = $q->{o};
+	my $l = $q->{l};
 	my $end = $o + $nr;
 	my $beg = $o + 1;
 	my $rv = '</pre><hr><pre id=t>';
@@ -191,7 +192,7 @@ sub search_nav_bot {
 	} else {
 		$rv .= "No more results, only $total";
 	}
-	my $n = $o + $LIM;
+	my $n = $o + $l;
 
 	if ($n < $total) {
 		my $qs = $q->qs_html(o => $n);
@@ -199,7 +200,7 @@ sub search_nav_bot {
 	}
 	if ($o > 0) {
 		$rv .= $n < $total ? '/' : '       ';
-		my $p = $o - $LIM;
+		my $p = $o - $l;
 		my $qs = $q->qs_html(o => ($p > 0 ? $p : 0));
 		$rv .= qq{<a\nhref="?$qs"\nrel=prev>prev</a>};
 	}
@@ -312,6 +313,7 @@ sub new {
 		q => $qp->{'q'},
 		x => $qp->{x} || '',
 		o => (($qp->{o} || '0') =~ /(\d+)/),
+		l => (($qp->{l} || '200') =~ /(\d+)/),
 		r => (defined $r && $r ne '0'),
 	}, $class;
 }
@@ -334,6 +336,9 @@ sub qs_html {
 	if (my $o = $self->{o}) { # ignore o == 0
 		$qs .= "&amp;o=$o";
 	}
+	if (my $l = $self->{l}) { 
+		$qs .= "&amp;l=$l";
+	}
 	if (my $r = $self->{r}) {
 		$qs .= "&amp;r";
 	}
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Allow specification of the number of search results to return
  2018-03-26 22:34 [PATCH] Allow specification of the number of search results to return Jonathan Corbet
@ 2018-03-27 10:44 ` Eric Wong
  2018-03-28 21:00   ` Jonathan Corbet
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Wong @ 2018-03-27 10:44 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: meta

Jonathan Corbet <corbet@lwn.net> wrote:
> Add an "l=" parameter to the search query syntax to specify how many
> results should be returned.  The default remains 200.
> ---
> 200 is a lot of results for one page, so allow it to be tweaked.  I've not
> added anything to the query form to access this (don't need it) but could.

There's the "feedmax" config variable, perhaps a config option
along those lines can be used with search results.

> --- a/lib/PublicInbox/SearchView.pm
> +++ b/lib/PublicInbox/SearchView.pm
> @@ -35,7 +35,7 @@ sub sres_top_html {
>  	my $code = 200;
>  	# double the limit for expanded views:
>  	my $opts = {
> -		limit => $LIM,
> +		limit => $q->{l},

Blindly trusting user input here could cause memory problems on
the server and should be clamped to a reasonable value.


Though I should note we support downloading entire mboxes of
search results.  However those are streamed via repeated queries
and public-inbox-httpd can handle those without loading lots of
data up front.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Allow specification of the number of search results to return
  2018-03-27 10:44 ` Eric Wong
@ 2018-03-28 21:00   ` Jonathan Corbet
  2018-03-28 21:40     ` Eric Wong
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Corbet @ 2018-03-28 21:00 UTC (permalink / raw)
  To: Eric Wong; +Cc: meta

On Tue, 27 Mar 2018 10:44:57 +0000
Eric Wong <e@80x24.org> wrote:

> Jonathan Corbet <corbet@lwn.net> wrote:
> > Add an "l=" parameter to the search query syntax to specify how many
> > results should be returned.  The default remains 200.
> > ---
> > 200 is a lot of results for one page, so allow it to be tweaked.  I've not
> > added anything to the query form to access this (don't need it) but could.  
> 
> There's the "feedmax" config variable, perhaps a config option
> along those lines can be used with search results.

That would solve my problem nicely, but takes the control out of the hands
of the reader.  Is this the approach you would rather see?

> > --- a/lib/PublicInbox/SearchView.pm
> > +++ b/lib/PublicInbox/SearchView.pm
> > @@ -35,7 +35,7 @@ sub sres_top_html {
> >  	my $code = 200;
> >  	# double the limit for expanded views:
> >  	my $opts = {
> > -		limit => $LIM,
> > +		limit => $q->{l},  
> 
> Blindly trusting user input here could cause memory problems on
> the server and should be clamped to a reasonable value.

Yeah...I saw it sort of like the threat from doing a lot of simultaneous
searches.  It's better to limit it, though; I can certainly do that if you
don't prefer the config-option approach.

Thanks,

jon

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Allow specification of the number of search results to return
  2018-03-28 21:00   ` Jonathan Corbet
@ 2018-03-28 21:40     ` Eric Wong
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2018-03-28 21:40 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: meta

Jonathan Corbet <corbet@lwn.net> wrote:
> On Tue, 27 Mar 2018 10:44:57 +0000
> Eric Wong <e@80x24.org> wrote:
> > Jonathan Corbet <corbet@lwn.net> wrote:
> > > Add an "l=" parameter to the search query syntax to specify how many
> > > results should be returned.  The default remains 200.
> > > ---
> > > 200 is a lot of results for one page, so allow it to be tweaked.  I've not
> > > added anything to the query form to access this (don't need it) but could.  
> > 
> > There's the "feedmax" config variable, perhaps a config option
> > along those lines can be used with search results.
> 
> That would solve my problem nicely, but takes the control out of the hands
> of the reader.  Is this the approach you would rather see?

I think we can support both a user-specified "l=" AND have a
configurable max as a safety measure.

> > Blindly trusting user input here could cause memory problems on
> > the server and should be clamped to a reasonable value.
> 
> Yeah...I saw it sort of like the threat from doing a lot of simultaneous
> searches.  It's better to limit it, though; I can certainly do that if you
> don't prefer the config-option approach.

Please do, thanks.

Several smaller, simultaneous searches is less overhead on the
server than one giant one since as it allows fairer scheduling.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-03-28 21:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-26 22:34 [PATCH] Allow specification of the number of search results to return Jonathan Corbet
2018-03-27 10:44 ` Eric Wong
2018-03-28 21:00   ` Jonathan Corbet
2018-03-28 21:40     ` Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).