Re: [aur-dev] [PATCH] Make external links in comments clickable (FS#20137).
On 10/01/2010 02:15 PM, PyroPeter wrote: I did not mean to offend you, and after applying the patch (which I should have done before sending the mails, you are right) your code in fact seems to work a lot better then I thought. While testing, I found a bug: Post this URL: http://foo.bar/lt;gt;lt;gt; It seems to trigger two bugs at once, first, the regex does not match whole URL, and second, the href is escaped twice. What I ment was the URL http://foo.bar/; Also, s/whole/the whole/ Regards, PyroPeter -- freenode/pyropeter 12:50 - Ich drücke Return.
Re: [aur-dev] [PATCH] Make external links in comments clickable (FS#20137).
On Fri, Oct 01, 2010 at 02:15:41PM +0200, PyroPeter wrote: I did not mean to offend you, and after applying the patch (which I should have done before sending the mails, you are right) your code in fact seems to work a lot better then I thought. I didn't feel offended in any way, but reporting bugs that don't exist is just counterproductive and a waste of time. While testing, I found a bug: Post this URL: http://foo.bar/lt;gt;lt;gt; It seems to trigger two bugs at once, first, the regex does not match whole URL, and second, the href is escaped twice. I already said that there might be problems if the URL contains quotes or less-than/greater-than symbols in another mail [1]. This can be fixed by removing the first str_replace() (which I also proposed in the same mail) or by repeating the second str_replace() for quot;, #039;, lt; and gt; (which might be even better from the perspective of security). However, I don't think such URLs will be a common use case. The second bug is expected behaviour, since punctuation marks at the end of URLs shouldn't be included in the URL itself (imagine someone putting a link at the end of a senctence). This is also how DokuWiki and Flyspray behave. If there really is an URL requiring a punctuation mark at the end of the URL (which there shouldn't be at all), this can be remarked in the comment itself. [1] http://mailman.archlinux.org/pipermail/aur-dev/2010-September/001263.html
Re: [aur-dev] AUR update
On Thu, Sep 30, 2010 at 09:54:00PM +0200, PyroPeter wrote: I tried to do something but i failed. i removed this completelly and changed all the password and so but now i'm completelly disolated.. There were no passwords in that file. (They are saved in /etc/shadow) Without that file your server won't boot. There are passwords in other files (especially web applications that often have MySQL passwords and stuff like that in config files) that are readable by the user the web server is being run as.
Re: [aur-dev] [PATCH] Make external links in comments clickable (FS#20137).
On 09/30/2010 05:22 PM, Lukas Fleischer wrote:
This is a bit hacky patch to make links in AUR comments clickable (fixes
FS#20137 [1]). Huge parts of this code are ripped from the DokuWiki
plugin that is also used in Flyspray.
I didn't have any time to test it extensively so I'd suggest to do some
more tests if this will be commited.
I fixed the / bugs and changed the indention to tabs.
I also changed the regex to one that accepts everything
that starts with one of 'http', 'https' or 'ftp' followed by
a colon :, contains no whitespace and ends with a letter (\w)
or a slash /.
It also parses hostnames starting with www. and ending
in a 2 to 5 letters long TLD consisting of only a-z.
I have tested it with the comments at [0].
Regards, PyroPeter
[0] http://aur.keks.selfip.org/packages.php?ID=298
---
diff --git a/web/lib/aur.inc b/web/lib/aur.inc
index bd69c4c..7465fac 100644
--- a/web/lib/aur.inc
+++ b/web/lib/aur.inc
@@ -494,3 +494,24 @@ function salted_hash($passwd, $salt)
}
return md5($salt . $passwd);
}
+
+function parse_link($matches)
+{
+ $name = $matches[0];
+ $url = preg_replace('/^www\./', 'http://www.', $matches[0]);
+ return 'a href=' . $url . '' . $name . '/a';
+}
+
+function parse_comment($comment)
+{
+ $regex = '/' . str_replace('/', '\/', implode('|',array(
+ '\b(http|https|ftp):[^\s]*[\w\d/]',
+ '\bwww\.[^\s/]+\.[a-z]{2,5}'
+ ))) . '/S';
+
+ $comment = htmlspecialchars($comment);
+ $comment = preg_replace_callback($regex, 'parse_link', $comment);
+ $comment = nl2br($comment);
+
+ return $comment;
+}
diff --git a/web/template/pkg_comments.php b/web/template/pkg_comments.php
index 02171a0..2ca9bf0 100644
--- a/web/template/pkg_comments.php
+++ b/web/template/pkg_comments.php
@@ -20,7 +20,7 @@ while (list($indx, $carr) = each($comments)) { ?
?/div
blockquote class=comment-body
div
-?php echo nl2br(htmlspecialchars($carr['Comments'])) ?
+?php echo parse_comment($carr['Comments']) ?
/div
/blockquote
?php
--
freenode/pyropeter 12:50 - Ich drücke Return.
Re: [aur-dev] [PATCH] Make external links in comments clickable (FS#20137).
On Fri, Oct 01, 2010 at 04:59:22PM +0200, PyroPeter wrote: I fixed the / bugs and changed the indention to tabs. I also changed the regex to one that accepts everything that starts with one of 'http', 'https' or 'ftp' followed by a colon :, contains no whitespace and ends with a letter (\w) or a slash /. It also parses hostnames starting with www. and ending in a 2 to 5 letters long TLD consisting of only a-z. [...] This won't match URLs like https://aur.archlinux.org/packages.php?O=0K=; and an ampersand at the end of an URL won't be converted correctly :/ I'll try to implement it a more proper way the next days. Maybe I'll actually go with splitting comments at link boundaries as you suggested before... :)
Re: [aur-dev] [PATCH] Make external links in comments clickable (FS#20137).
On 10/01/2010 05:52 PM, Lukas Fleischer wrote: This won't match URLs like https://aur.archlinux.org/packages.php?O=0K=; and an ampersand at the end of an URL won't be converted correctly :/ I'll try to implement it a more proper way the next days. Maybe I'll actually go with splitting comments at link boundaries as you suggested before... :) Well, that's the problem. Which characters should belong to the end of the URL, and which should not? There could also be cases in which punctuation belongs to the URL. If punctuation is parsed as not belonging to the URL, there would be no way to post a working link to certain URLs. If punctuation is parsed as part of the URL, one could insert a space between the URL and the punctuation that should not belong to the URL. One should also consider that inserting an URL into a sentence looks horrible and is normally not done (by me, at least). About splitting at boundaries: Contrary to what I have said before, using regular expressions seems to be a valid and efficient way. (I thought you would have to escape tag-content and attributes in different ways (percent-encoding vs. html-entities). After reading the HTML4 specification I realized this is not the case, as content and attributes are both escaped using html-entities) Regards, PyroPeter -- freenode/pyropeter 12:50 - Ich drücke Return.
Re: [aur-dev] [PATCH] Make external links in comments clickable (FS#20137).
On Fri, Oct 01, 2010 at 06:23:06PM +0200, PyroPeter wrote: About splitting at boundaries: Contrary to what I have said before, using regular expressions seems to be a valid and efficient way. (I thought you would have to escape tag-content and attributes in different ways (percent-encoding vs. html-entities). After reading the HTML4 specification I realized this is not the case, as content and attributes are both escaped using html-entities) Using regular expressions is an efficient way, but they should be applied before htmlspecialchars() or anything similar is applied. E.g. we could use preg_match() or preg_match_all() with PREG_OFFSET_CAPTURE to get the positions of all links, then call a function, that converts links and converts the stings as necessary, and convert the parts that don't contain any links separately using htmlspecialchars().
Re: [aur-dev] [PATCH] Make external links in comments clickable (FS#20137).
On Fri, Oct 01, 2010 at 06:23:06PM +0200, PyroPeter wrote: On 10/01/2010 05:52 PM, Lukas Fleischer wrote: This won't match URLs like https://aur.archlinux.org/packages.php?O=0K=; and an ampersand at the end of an URL won't be converted correctly :/ I'll try to implement it a more proper way the next days. Maybe I'll actually go with splitting comments at link boundaries as you suggested before... :) Well, that's the problem. Which characters should belong to the end of the URL, and which should not? There could also be cases in which punctuation belongs to the URL. If punctuation is parsed as not belonging to the URL, there would be no way to post a working link to certain URLs. If punctuation is parsed as part of the URL, one could insert a space between the URL and the punctuation that should not belong to the URL. One should also consider that inserting an URL into a sentence looks horrible and is normally not done (by me, at least). About splitting at boundaries: Contrary to what I have said before, using regular expressions seems to be a valid and efficient way. (I thought you would have to escape tag-content and attributes in different ways (percent-encoding vs. html-entities). After reading the HTML4 specification I realized this is not the case, as content and attributes are both escaped using html-entities) Regards, PyroPeter -- freenode/pyropeter 12:50 - Ich drücke Return. I didn't read the whole thread but as far as I understand you're searching for a proper solution how to correctly find urls in comments. John Gruber's Regex seems quite right for this: http://daringfireball.net/2010/07/improved_regex_for_matching_urls Does this help? Jan-Erik (badboy_)
Re: [aur-dev] [PATCH] Geshi AUR implementation
On Thu 30 Sep 2010 20:13 +0200, Lukas Fleischer wrote: On Wed, Sep 29, 2010 at 03:35:24PM +0200, Manuel Tortosa wrote: This introduces a remote file inclusion vulnerability allowing an attacker to read arbitrary files since $pkgbuild is not validated before passing it to file_get_contents(). Don't apply this patch until everything is fixed, please. Thanks for your suggestions, i added them all to CCR ;) Btw, this is still not fixed! Have a look at [1]. You should consider using basename(), realpath() and/or regexp to check the PKGBUILD path. Also check [2], [3]. [1] http://mailman.archlinux.org/pipermail/aur-dev/2010-September/001268.html [2] http://www.madirish.net/?article=427 [3] http://www.acunetix.com/websitesecurity/php-security-3.htm Thanks for helping review these patches Lukas. It's much appreciated.
Re: [aur-dev] [PATCH] Minor fix in French translation (FS#20111).
On Thu 30 Sep 2010 17:42 +0200, Lukas Fleischer wrote: --- web/lang/fr.po |8 1 files changed, 4 insertions(+), 4 deletions(-) ... $_t[Search Criteria] = Critères de recherche; I decided to close that bug as won't implement. Also I notice that you're sending your mails as charset=iso-8859-1 not utf-8, which is causing problems with the encodings. Please check your mail client settings. You might also gzip patches if you can't send in the correct encoding. Cheers.
Re: [aur-dev] [PATCH] Add timestamp when a package is flagged out-of-date (FS#20848).
On Thu 30 Sep 2010 18:19 +0200, Lukas Fleischer wrote: --- UPGRADING |4 support/schema/aur-schema.sql |1 + web/lang/cs.po|2 +- web/lang/de.po|2 +- web/lang/el_GR.po |2 +- web/lang/fr.po|2 +- web/lang/he.po|2 +- web/lang/hr.po|2 +- web/lang/hu.po|2 +- web/lang/it.po|2 +- web/lang/nb_NO.po |2 +- web/lang/pl.po|2 +- web/lang/ro.po|2 +- web/lang/ru.po|2 +- web/lang/sr.po|2 +- web/lang/tr.po|2 +- web/lang/uk.po|2 +- web/lang/zh_CN.po |2 +- web/lib/pkgfuncs.inc |3 ++- web/template/pkg_details.php |3 ++- 20 files changed, 25 insertions(+), 18 deletions(-) diff --git a/UPGRADING b/UPGRADING index 743f404..99c5d1a 100644 --- a/UPGRADING +++ b/UPGRADING @@ -1,6 +1,10 @@ Upgrading = +From 1.7.0 +-- +ALTER TABLE Packages ADD OutOfDateTS BIGINT UNSIGNED NOT NULL; + From 1.6.0 to 1.7.0 --- ALTER TABLE Users ADD Salt CHAR(32) NOT NULL DEFAULT ''; diff --git a/support/schema/aur-schema.sql b/support/schema/aur-schema.sql index 250d405..15705a3 100644 --- a/support/schema/aur-schema.sql +++ b/support/schema/aur-schema.sql @@ -120,6 +120,7 @@ CREATE TABLE Packages ( LocationID TINYINT UNSIGNED NOT NULL DEFAULT 1, NumVotes INTEGER UNSIGNED NOT NULL DEFAULT 0, OutOfDate TINYINT UNSIGNED DEFAULT 0, + OutOfDateTS BIGINT UNSIGNED NOT NULL, SubmittedTS BIGINT UNSIGNED NOT NULL, ModifiedTS BIGINT UNSIGNED NOT NULL, SubmitterUID INTEGER UNSIGNED NOT NULL DEFAULT 0, -- who submitted it? I was thinking - could we just change things so that OutOfDateTS is the sole indicator of whether a package has been flagged or not? If it's set, then it's out of date. If it's zero or null, then it's up to date. This might also apply to deletion, or orphan requests. Really all these types of flagging could share the same structure and code as far as I can envision it. Also, I think that all these translation changes are really unnecessary.
