On 8/31/06, martin f krafft <[EMAIL PROTECTED]> wrote:
also sprach Thibaut VARENE <[EMAIL PROTECTED]> [2006.08.31.0859 +0200]:
> I don't understand what's wrong. You underlined the UTF8 string, which
> looks ok to me...

The ? must be escaped in URLs, and any & as well.

There's no escape sequence for '?'. It's '&' at fault here. I
unfortunately introduced this bug trying to fix another one: xmms
apparently won't play files in playlists containing "&amp;".
Considering what I just read on w3.org, I suppose that's a bug in
xmms. ;)

Anyway, I cooked the attached patch. If you have some time for
debugging, I'd welcome you try it with as many files as you can,
especially as nasty filenames as you can, with as many
players/browsers/aggregators as you can, and let me know how things
go. I expect it'll break xmms. If you use it and it does break it,
feel free to send a bugreport against it :)

It's a quick and dirty sed, plus a couple fix for potential similar
issues with CSS in send_head. I haven't even checked it builds, I hope
it does :)

Thanks a lot

T-Bone

--
Thibaut VARENE
http://www.parisc-linux.org/~varenet/
--- mod_musicindex/src/html.c.orig	2006-08-01 01:30:15.000000000 +0200
+++ mod_musicindex/src/html.c	2006-08-31 11:50:07.000000000 +0200
@@ -199,7 +199,7 @@
 				if ((current[0] == '\0') || (current[1] == '\0'))	/* current dir is either "" or "/" */
 					ap_rputs(_("In Current Directory"), r);
 				else
-					ap_rvputs(r, _("In "), "<a href=\"", ap_escape_uri(r->pool, current), "\">", ap_escape_html(r->pool, current), "</a>", NULL);
+					ap_rvputs(r, _("In "), "<a href=\"", ap_escape_html(r->pool, ap_escape_uri(r->pool, current)), "\">", ap_escape_html(r->pool, current), "</a>", NULL);
 
 				ap_rputs("</th>\n"
 					"  </tr>\n", r);
@@ -229,12 +229,12 @@
 
 			if (customlist == 0) {
 				if (q->flags & EF_ALLOWDWNLD)	/* Display [download] */
-					ap_rvputs(r, "    <a href=\"", ap_escape_uri(r->pool, q->file), "\">"
+					ap_rvputs(r, "    <a href=\"", ap_escape_html(r->pool, ap_escape_uri(r->pool, q->file)), "\">"
 						"<img alt=\"[D]\" title=\"", _("Download"), "\" src=\"", conf->directory, "/",
 						Gfetch_icon, "\" /></a>\n", NULL);
 
 				if (q->flags & EF_ALLOWSTREAM)	/* Display [stream] */
-					ap_rvputs(r, "    <a href=\"", ap_escape_uri(r->pool, q->file), "?stream\">"
+					ap_rvputs(r, "    <a href=\"", ap_escape_html(r->pool, ap_escape_uri(r->pool, q->file)), "?stream\">"
 						"<img alt=\"[S]\" title=\"", _("Stream"), "\" src=\"", conf->directory, "/",
 						Gsound_icon, "\" /></a>\n", NULL);
 			}
@@ -379,7 +379,7 @@
 	}
 
 	/* add the uri and potential command */
-	ap_rvputs(r, prefix, ap_escape_uri(r->pool, uri), NULL);
+	ap_rvputs(r, prefix, ap_escape_html(r->pool, ap_escape_uri(r->pool, uri)), NULL);
 	if (command)
 		ap_rputs(command, r);
 }
@@ -392,6 +392,8 @@
  *
  * @param r Apache request_rec struct to handle connection details.
  * @param conf MusicIndex configuration paramaters struct.
+ * 
+ * @bug When we'll allow conf->directory to be modified, have to escape html/uri.
  */
 void send_head(request_rec *r, const mu_config *const conf)
 {
@@ -429,9 +431,9 @@
 					if (!strcmp(dstruct->d_name, conf->css))
 						ap_rputs(" <link rel=\"stylesheet\" title=\"default\"", r);
 					else
-						ap_rvputs(r, " <link rel=\"alternate stylesheet\" title=\"", dstruct->d_name, "\"", NULL);
+						ap_rvputs(r, " <link rel=\"alternate stylesheet\" title=\"", ap_escape_html(r->pool, ap_escape_uri(r->pool, dstruct->d_name)), "\"", NULL);
 
-					ap_rvputs(r, " type=\"text/css\" href=\"", conf->directory, "/", dstruct->d_name, "\" />\n", NULL);
+					ap_rvputs(r, " type=\"text/css\" href=\"", conf->directory, "/", ap_escape_html(r->pool, ap_escape_uri(r->pool, dstruct->d_name)), "\" />\n", NULL);
 				}
 			}
 			closedir(dir);
@@ -515,7 +517,7 @@
 				dir = localconf->title;
 
 			*u = '\0';
-			ap_rvputs(r, "   <a href=\"", ap_escape_uri(r->pool, uri), "/\">", ap_escape_html(r->pool, dir), "</a>\n", NULL);
+			ap_rvputs(r, "   <a href=\"", ap_escape_html(r->pool, ap_escape_uri(r->pool, uri)), "/\">", ap_escape_html(r->pool, dir), "</a>\n", NULL);
 			*u = '/';
 
 			if (*(u+1) != '\0')
@@ -587,7 +589,7 @@
 	/* XXX collision avec le header, a regler */
  	if (conf->options & MI_ALLOWSEARCH) {
 		ap_rvputs(r,
-			" <form method=\"post\" action=\"", ap_escape_uri(r->pool, r->uri), "\""
+			" <form method=\"post\" action=\"", ap_escape_html(r->pool, ap_escape_uri(r->pool, r->uri)), "\""
 				" enctype=\"application/x-www-form-urlencoded\""
 				" id=\"searching\">\n"
 			"  <p>\n"
@@ -658,7 +660,7 @@
 			ap_rputs(" <tr>\n", r);
 
 		ap_rvputs(r, "  <td>\n"
-			"   <a href=\"", ap_escape_uri(r->pool, q->file), NULL);
+			"   <a href=\"", ap_escape_html(r->pool, ap_escape_uri(r->pool, q->file)), NULL);
 
 		ap_rputs("\"><img alt=\"\" src=\"", r);
 #ifdef SHOW_THUMBNAILS
@@ -679,20 +681,20 @@
 		ap_rputs("\" /></a>\n", r);
 
 		ap_rvputs(r, "   <div>\n"
-			"    <a href=\"", ap_escape_uri(r->pool, q->file), "\">",
+			"    <a href=\"", ap_escape_html(r->pool, ap_escape_uri(r->pool, q->file)), "\">",
 	    		temp, "</a><br />\n", NULL);
 
 		/* show various useful links when needed */
 		if (q->flags & EF_ALLOWSTREAM) {
 			ap_rvputs(r, "    <a class=\"shuffle\" href=\"",
-				ap_escape_uri(r->pool, q->file),
+				ap_escape_html(r->pool, ap_escape_uri(r->pool, q->file)),
 				"?option=recursive&amp;option=shuffle&amp;action=playall\">",
 				"<img alt=\"[R]\" title=\"", _("Shuffle"), "\" src=\"",
 				conf->directory, "/", Gshuffle_icon,
 				"\" /></a>\n", NULL);
 
 			ap_rvputs(r, "    <a class=\"stream\" href=\"",
-				ap_escape_uri(r->pool, q->file),
+				ap_escape_html(r->pool, ap_escape_uri(r->pool, q->file)),
 				"?option=recursive&amp;action=playall\">",
 				"<img alt=\"[S]\" title=\"", _("Stream"), "\" src=\"",
 				conf->directory, "/", Gsound_icon,
@@ -701,7 +703,7 @@
 
 		if (q->flags & EF_ALLOWTARBALL) {
 			ap_rvputs(r, "    <a class=\"tarball\" href=\"",
-				ap_escape_uri(r->pool, q->file),
+				ap_escape_html(r->pool, ap_escape_uri(r->pool, q->file)),
 				"?option=recursive&amp;action=tarball\">",
 				"<img alt=\"[D]\" title=\"", _("Download"), "\" src=\"",
 				conf->directory, "/", Gfetch_icon,
@@ -710,7 +712,7 @@
 
 		if (q->flags & EF_ALLOWRSS) {
 			ap_rvputs(r, "    <a class=\"rss\" href=\"",
-				ap_escape_uri(r->pool, q->file),
+				ap_escape_html(r->pool, ap_escape_uri(r->pool, q->file)),
 				"?action=RSS\">",
 				"<img alt=\"[RSS]\" title=\"", _("RSS"), "\" src=\"",
 				conf->directory, "/", Grss_icon,
@@ -769,7 +771,7 @@
 
 	ap_rputs("</h2>\n\n", r);
 
-	ap_rvputs(r, "<form method=\"post\" action=\"", ap_escape_uri(r->pool, r->uri), "\" "
+	ap_rvputs(r, "<form method=\"post\" action=\"", ap_escape_html(r->pool, ap_escape_uri(r->pool, r->uri)), "\" "
 		"enctype=\"application/x-www-form-urlencoded\" id=\"tracks\">\n", NULL);
 
 	ap_rputs(" <table>\n", r);
@@ -844,7 +846,7 @@
 	ap_rprintf(r, _("Custom Playlist (%d)"), nb);
 	ap_rputs("</h2>\n\n", r);
 
-	ap_rvputs(r, " <form method=\"post\" action=\"", ap_escape_uri(r->pool, r->uri), "\" "
+	ap_rvputs(r, " <form method=\"post\" action=\"", ap_escape_html(r->pool, ap_escape_uri(r->pool, r->uri)), "\" "
 		"enctype=\"application/x-www-form-urlencoded\" id=\"custom\">\n", NULL);
 
 	ap_rputs("  <table>\n", r);

Reply via email to