Bug: membership file too raw in stats page

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug: membership file too raw in stats page

Phil Pennock-17
I've not got time right now to get back into picking my way through
o'caml and the sks code, so I'll just report the issue in the hopes that
someone like Kim comes up with a fix.  :)

One of my peers had a line in their membership file which followed the
common idiom of:
  host.example.net 11370    # Fred Bloggs <[hidden email]>

Unfortunately, they'd missed the '#' character.  The
/pks/lookup?op=stats page returned this raw with no HTML entity
escaping.

I noticed while debugging a fix for my sks-peers.py WSGI app (one of the
dependencies is going through API changes and I'd missed one), since
this meant that the Python BeautifulSoup HTML parser failed to parse the
page.

The stats renderer should be HTML-escaping the data it's putting into
fields in HTML.  And hey, if it's not already doing so, complaining more
loudly about such lines at parse time so the admins notice.  :)

Regards,
-Phil

_______________________________________________
Sks-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/sks-devel

attachment0 (169 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Bug: membership file too raw in stats page

Kim Minh Kaplan
Phil Pennock writes:

> One of my peers had a line in their membership file which followed the
> common idiom of:
>   host.example.net 11370    # Fred Bloggs <[hidden email]>
>
> Unfortunately, they'd missed the '#' character.  The
> /pks/lookup?op=stats page returned this raw with no HTML entity
> escaping.

Although SKS could be more carefull with the escaping on the stats page
I can not reproduce the problem you describe.  The peers table is built
from the result of Membership.get_names() which uses a Scanf "%s %s".
Only the hostname and service are used, the rest of the line is silently
discarded.

> The stats renderer should be HTML-escaping the data it's putting into
> fields in HTML.  And hey, if it's not already doing so, complaining more
> loudly about such lines at parse time so the admins notice.  :)

Patches attached for each.  Should I push them to my repository so that
they get integrated in mainline?

Kim Minh.


Fix stats page HTML entities encoding.
http://lists.gnu.org/archive/html/sks-devel/2009-10/msg00001.html

diff -r 95926d02fe80 stats.ml
--- a/stats.ml Mon Aug 31 23:33:41 2009 +0200
+++ b/stats.ml Mon Oct 12 21:42:52 2009 +0200
@@ -139,12 +139,14 @@
      <tr><td>Recon port:<td>%d
      <tr><td>Debug level:<td>%d
 </table><br>"
-      !Settings.hostname Common.version
+      (HtmlTemplates.html_quote !Settings.hostname)
+      (HtmlTemplates.html_quote Common.version)
       http_port recon_port !Settings.debuglevel
   in
   let gossip_peers =
     let peers = Array.to_list (Membership.get_names ()) in
-    let peers = List.map ~f:(fun peer -> sprintf "<tr><td>%s\n" peer) peers in
+    let peers = List.rev_map ~f:HtmlTemplates.html_quote peers in
+    let peers = List.rev_map ~f:(sprintf "<tr><td>%s\n") peers in
     sprintf "<h2>Gossip Peers</h2>\n<table>\n%s</table>"
       (String.concat ~sep:"" peers)
   in
@@ -153,7 +155,8 @@
       try Membership.get_mailsync_partners ()
       with Failure "No partners specified" -> []
     in
-    let peers = List.map ~f:(fun s -> sprintf "<tr><td>%s\n" s) peers in
+    let peers = List.rev_map ~f:HtmlTemplates.html_quote peers in
+    let peers = List.rev_map ~f:(sprintf "<tr><td>%s\n") peers in
     sprintf "<h2>Outgoing Mailsync Peers</h2>\n<table>\n%s</table>"
       (String.concat ~sep:"" peers)
   in

Complain on malformed membership lines.
http://lists.gnu.org/archive/html/sks-devel/2009-10/msg00001.html

diff -r 72d8e25ca99a membership.ml
--- a/membership.ml Mon Oct 12 21:42:52 2009 +0200
+++ b/membership.ml Mon Oct 12 21:57:13 2009 +0200
@@ -46,9 +46,11 @@
 let convert_address l =
   try
     if String.length l = 0 then raise Empty_line else
-    sscanf l "%s %s"
-      (fun addr service ->
+    sscanf l "%s %s %s"
+      (fun addr service rest ->
          if addr = "" || service = "" then failwith "Blank line";
+ if rest <> "" then
+   perror "Spurious content \"%s\" in membership line \"%s\"" rest l;
          addr, service)
   with
     Scanf.Scan_failure _ | End_of_file | Failure _ -> raise (Malformed_entry l)

_______________________________________________
Sks-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/sks-devel
Reply | Threaded
Open this post in threaded view
|

Re: Bug: membership file too raw in stats page

Phil Pennock-17
On 2009-10-12 at 20:08 +0000, Kim Minh Kaplan wrote:

> Phil Pennock writes:
>
> > One of my peers had a line in their membership file which followed the
> > common idiom of:
> >   host.example.net 11370    # Fred Bloggs <[hidden email]>
> >
> > Unfortunately, they'd missed the '#' character.  The
> > /pks/lookup?op=stats page returned this raw with no HTML entity
> > escaping.
>
> Although SKS could be more carefull with the escaping on the stats page
> I can not reproduce the problem you describe.  The peers table is built
> from the result of Membership.get_names() which uses a Scanf "%s %s".
> Only the hostname and service are used, the rest of the line is silently
> discarded.
http://keyserver.stack.nl:11371/pks/lookup?op=stats

It's running 1.1.0; that's using conver_address with a sscanf "%s %d";
however, membership holds (mshp, mtime) and mshp is an Array of tuples
where the first element is addr (host port) and the second is the line,
after comments have been removed.

Then Membership.get_names() is doing:
   Array.map ~f:snd mshp
to just return the lines, not the parsed data.  So as long as the line
*starts* "%s %d" then it will be stored.

Regards,
-Phil

_______________________________________________
Sks-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/sks-devel

attachment0 (169 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Bug: membership file too raw in stats page

Kim Minh Kaplan
Phil Pennock writes:

> It's running 1.1.0;

That's the reason I could not reproduce: this has changed in 1.1.1 where
only the host and port are reported on the stats page.

Kim Minh.


_______________________________________________
Sks-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/sks-devel