From 353ad2afd0dc3cf588b7750055af3f36e5941c35 Mon Sep 17 00:00:00 2001 From: Jeremy Edberg Date: Tue, 22 Jul 2008 13:37:37 -0700 Subject: [PATCH] improved url handling --- r2/r2/config/routing.py | 19 +++++------ r2/r2/controllers/front.py | 41 +++++++++++++++++++----- r2/r2/controllers/listingcontroller.py | 2 +- r2/r2/controllers/validator/validator.py | 2 +- r2/r2/lib/emailer.py | 27 +++++++++------- r2/r2/lib/menus.py | 2 ++ r2/r2/lib/pages/pages.py | 39 ++++++++++++++-------- r2/r2/lib/utils/utils.py | 21 +++++++++++- r2/r2/models/link.py | 8 ++--- r2/r2/public/static/link.js | 4 +-- r2/r2/public/static/robots.txt | 4 +-- r2/r2/templates/base.html | 15 ++++++++- r2/r2/templates/comment.html | 2 +- r2/r2/templates/link.html | 2 ++ r2/r2/templates/link.htmllite | 6 ++-- r2/r2/templates/listing.html | 4 +-- r2/r2/templates/printable.html | 9 +++--- r2/r2/templates/reddit.html | 2 +- r2/r2/templates/redditfooter.html | 6 ++-- r2/r2/templates/share.email | 6 +--- r2/supervise_watcher.py | 2 +- 21 files changed, 147 insertions(+), 76 deletions(-) diff --git a/r2/r2/config/routing.py b/r2/r2/config/routing.py index 025681a6a..c73a4f271 100644 --- a/r2/r2/config/routing.py +++ b/r2/r2/config/routing.py @@ -80,16 +80,11 @@ def make_map(global_conf={}, app_conf={}): mc('/prefs/:location', controller='front', action='prefs', location='options') - mc('/info/0:name/*rest', controller = 'front', action='oldinfo') + mc('/info/0:article/*rest', controller = 'front', + action='oldinfo', dest='comments', type='ancient') + mc('/info/:article/:dest/:comment', controller='front', + action='oldinfo', type='old', dest='comments', comment=None) - mc('/info/:article/comments/:comment', - controller='front', action='comments', - comment = None) - mc('/info/:article/related', controller='front', - action = 'related') - mc('/info/:article/details', controller='front', - action = 'details') - mc('/related/:article/:title', controller='front', action = 'related', title=None) mc('/details/:article/:title', controller='front', @@ -138,6 +133,12 @@ def make_map(global_conf={}, app_conf={}): mc('/store', controller='redirect', action='redirect', dest='http://store.reddit.com/index.html') + mc('/code', controller='redirect', action='redirect', + dest='http://code.reddit.com/') + + mc('/mobile', controller='redirect', action='redirect', + dest='http://m.reddit.com/') + # This route handles displaying the error page and # graphics used in the 404/500 # error pages. It should likely stay at the top diff --git a/r2/r2/controllers/front.py b/r2/r2/controllers/front.py index 60a8be039..a260f7925 100644 --- a/r2/r2/controllers/front.py +++ b/r2/r2/controllers/front.py @@ -27,8 +27,8 @@ from r2 import config from r2.models import * from r2.lib.pages import * from r2.lib.menus import * +from r2.lib.utils import to36, sanitize_url, check_cheating, title_to_url, query_string from r2.lib.emailer import opt_out, opt_in -from r2.lib.utils import to36, sanitize_url, check_cheating from r2.lib.db.operators import desc from r2.lib.strings import strings import r2.lib.db.thing as thing @@ -37,17 +37,39 @@ from pylons import c, request import random as rand import re +from urllib import quote_plus from admin import admin_profile_query class FrontController(RedditController): - def GET_oldinfo(self, name, rest): - """Legacy: supporting permalink pages from '06""" - #this could go in config, but it should never change - max_link_id = 10000000 - new_id = max_link_id - int(name) - return self.redirect('/info/' + to36(new_id) + '/' + rest) + @validate(article = VLink('article'), + comment = VCommentID('comment')) + def GET_oldinfo(self, article, type, dest, rest=None, comment=''): + """Legacy: supporting permalink pages from '06, + and non-search-engine-friendly links""" + if not (dest in ('comments','related','details')): + dest = 'comments' + if type == 'ancient': + #this could go in config, but it should never change + max_link_id = 10000000 + new_id = max_link_id - int(article._id) + return self.redirect('/info/' + to36(new_id) + '/' + rest) + if type == 'old': + new_url = "/%s/%s/%s" % \ + (dest, article._id36, + quote_plus(title_to_url(article.title).encode('utf-8'))) + if not c.default_sr: + new_url = "/r/%s%s" % (c.site.name, new_url) + if comment: + new_url = new_url + "/%s" % comment._id36 + if request.environ.get('extension'): + new_url = new_url + "/.%s" % request.environ.get('extension') + + new_url = new_url + query_string(request.get) + + # redirect should be smarter and handle extentions, etc. + return self.redirect(new_url) def GET_random(self): """The Serendipity button""" @@ -95,6 +117,9 @@ class FrontController(RedditController): num_comments = VMenu('controller', NumCommentsMenu)) def GET_comments(self, article, comment, context, sort, num_comments): """Comment page for a given 'article'.""" + if comment and comment.link_id != article._id: + return self.abort404() + if not c.default_sr and c.site._id != article.sr_id: return self.abort404() @@ -140,7 +165,7 @@ class FrontController(RedditController): loc = None if c.focal_comment or context is not None else 'comments' - res = LinkInfoPage(link = article, + res = LinkInfoPage(link = article, comment = comment, content = displayPane, nav_menus = [CommentSortMenu(default = sort), NumCommentsMenu(article.num_comments, diff --git a/r2/r2/controllers/listingcontroller.py b/r2/r2/controllers/listingcontroller.py index b4e499067..ac96d581c 100644 --- a/r2/r2/controllers/listingcontroller.py +++ b/r2/r2/controllers/listingcontroller.py @@ -128,7 +128,7 @@ class ListingController(RedditController): def title(self): """Page """ - return c.site.name + ': ' + _(self.title_text) + return _(self.title_text) + " : " + c.site.name def rightbox(self): """Contents of the right box when rendering""" diff --git a/r2/r2/controllers/validator/validator.py b/r2/r2/controllers/validator/validator.py index d0343ea86..0e7cf5cc2 100644 --- a/r2/r2/controllers/validator/validator.py +++ b/r2/r2/controllers/validator/validator.py @@ -153,7 +153,7 @@ class VCommentID(Validator): cid = int(cid, 36) return Comment._byID(cid, True) except (NotFound, ValueError): - abort(404, 'page not found') + pass class VCount(Validator): def run(self, count): diff --git a/r2/r2/lib/emailer.py b/r2/r2/lib/emailer.py index 34a9235b6..3fd34c7cb 100644 --- a/r2/r2/lib/emailer.py +++ b/r2/r2/lib/emailer.py @@ -27,21 +27,18 @@ from r2.lib.utils import timeago from r2.models import passhash, Email, Default from r2.config import cache import os, random, datetime -import smtplib +import smtplib, traceback, sys def email_address(name, address): return '"%s" <%s>' % (name, address) if name else address feedback = email_address('reddit feedback', g.feedback_email) -def send_mail(msg, fr, to, test = False): - if not test: - session = smtplib.SMTP(g.smtp_server) - session.sendmail(fr, to, msg.as_string()) - session.quit() - else: - g.log.debug(msg.as_string()) +def send_mail(msg, fr, to): + session = smtplib.SMTP(g.smtp_server) + session.sendmail(fr, to, msg.as_string()) + session.quit() -def simple_email(to, fr, subj, body, test = False): +def simple_email(to, fr, subj, body): def utf8(s): return s.encode('utf8') if isinstance(s, unicode) else s msg = MIMEText(utf8(body)) @@ -49,7 +46,7 @@ def simple_email(to, fr, subj, body, test = False): msg['To'] = utf8(to) msg['From'] = utf8(fr) msg['Subject'] = utf8(subj) - send_mail(msg, fr, to, test = test) + send_mail(msg, fr, to) def sys_email(email, body, name='', subj = lambda x: x): fr = (c.user.name if c.user else 'Anonymous user') @@ -104,8 +101,14 @@ def send_queued_mail(): link = email.thing).render(style = "email") email.subject = _("[reddit] %(user)s has shared a link with you") % \ {"user": email.from_name()} - session.sendmail(email.fr_addr, email.to_addr, - email.to_MIMEText().as_string()) + try: + session.sendmail(email.fr_addr, email.to_addr, + email.to_MIMEText().as_string()) + except smtplib.SMTPRecipientsRefused: + # handle error but don't stop the queue + print "Handled error sending mail (traceback to follow)" + traceback.print_exc(file = sys.stdout) + elif email.kind == Email.Kind.OPTOUT: email.fr_addr = g.share_reply email.body = Mail_Opt(msg_hash = email.msg_hash, diff --git a/r2/r2/lib/menus.py b/r2/r2/lib/menus.py index 8c6642cbb..4120b7572 100644 --- a/r2/r2/lib/menus.py +++ b/r2/r2/lib/menus.py @@ -92,6 +92,8 @@ menu = MenuHandler(hot = _('hot'), bookmarklets = _("bookmarklets"), buttons = _("buttons"), widget = _("widget"), + code = _("code"), + mobile = _("mobile"), store = _("store"), ad_inq = _("advertise"), diff --git a/r2/r2/lib/pages/pages.py b/r2/r2/lib/pages/pages.py index 2673ef9f0..52f48e3ee 100644 --- a/r2/r2/lib/pages/pages.py +++ b/r2/r2/lib/pages/pages.py @@ -28,9 +28,10 @@ from pylons.i18n import _ from pylons import c, request, g from r2.lib.captcha import get_iden -from r2.lib.filters import spaceCompress +from r2.lib.filters import spaceCompress, _force_unicode from r2.lib.menus import NavButton, NamedButton, NavMenu, PageNameNav, JsButton, menu from r2.lib.strings import plurals, rand_strings, strings +from r2.lib.utils import title_to_url def get_captcha(): if not c.user_is_loggedin or c.user.needs_captcha(): @@ -66,10 +67,11 @@ class Reddit(Wrapped): extension_handling = True def __init__(self, space_compress = True, nav_menus = None, loginbox = True, - infotext = '', content = None, title = '', show_sidebar = True, - **context): + infotext = '', content = None, title = '', robots = None, + show_sidebar = True, **context): Wrapped.__init__(self, **context) self.title = title + self.robots = robots self.infotext = infotext self.loginbox = True self.show_sidebar = show_sidebar @@ -172,6 +174,8 @@ class Reddit(Wrapped): NamedButton("bookmarklets", False), NamedButton("buttons", False), NamedButton("widget", False), + NamedButton("code", False), + NamedButton("mobile", False), NamedButton("store", False), NamedButton("ad_inq", False), ] @@ -355,7 +359,7 @@ class SearchPage(BoringPage): self.searchbar = SearchBar(prev_search = prev_search, elapsed_time = elapsed_time, num_results = num_results) - BoringPage.__init__(self, pagename, *a, **kw) + BoringPage.__init__(self, pagename, robots='noindex', *a, **kw) def content(self): return self.content_stack(self.searchbar, self.infobar, @@ -374,7 +378,8 @@ class LinkInfoPage(Reddit): create_reddit_box = False - def __init__(self, link = None, title = '', *a, **kw): + def __init__(self, link = None, comment = None, + link_title = '', *a, **kw): # TODO: temp hack until we find place for builder_wrapper from r2.controllers.listingcontroller import ListingController link_builder = IDBuilder(link._fullname, wrap = ListingController.builder_wrapper) @@ -385,21 +390,29 @@ class LinkInfoPage(Reddit): # link is a wrapped Link object self.link = self.link_listing.things[0] - title = c.site.name + ((': ' + self.link.title) \ - if hasattr(self.link, 'title') else '') - + link_title = ((self.link.title) if hasattr(self.link, 'title') else '') + if comment: + author = Account._byID(comment.author_id, data=True).name + title = _("%(author)s comments on %(title)s") % dict(author=author, title=_force_unicode(link_title)) + else: + title = _("%(title)s : %(site)s") % dict(title=_force_unicode(link_title), site = c.site.name) Reddit.__init__(self, title = title, *a, **kw) def build_toolbars(self): - base_path = "/info/%s/" % self.link._id36 + base_path = "/%s/%s/" % (self.link._id36, title_to_url(self.link.title)) + if isinstance(base_path, unicode): + base_path = base_path.encode('utf-8') + def info_button(name): + return NamedButton(name, dest = '/%s%s' % (name, base_path), + aliases = ['/%s/%s' % (name, self.link._id36)]) - buttons = [NavButton(plurals.comments, 'comments'), - NamedButton('related')] + buttons = [info_button('comments'), + info_button('related')] if c.user_is_admin: - buttons += [NamedButton('details')] + buttons += [info_button('details')] - toolbar = [NavMenu(buttons, base_path = base_path, type="tabmenu")] + toolbar = [NavMenu(buttons, base_path = "", type="tabmenu")] if c.site != Default: toolbar.insert(0, PageNameNav('subreddit')) diff --git a/r2/r2/lib/utils/utils.py b/r2/r2/lib/utils/utils.py index 416f3b301..011cda27e 100644 --- a/r2/r2/lib/utils/utils.py +++ b/r2/r2/lib/utils/utils.py @@ -28,8 +28,8 @@ import cPickle as pickle import re, datetime, math, random, string, sha from datetime import datetime, timedelta - from pylons.i18n import ungettext, _ +from r2.lib.filters import _force_unicode iters = (list, tuple, set) @@ -744,3 +744,22 @@ def valid_vote_hash(hash, user, thing): def safe_eval_str(unsafe_str): return unsafe_str.replace('\\x3d', '=').replace('\\x26', '&') + +rx_whitespace = re.compile('\s+', re.UNICODE) +rx_notsafe = re.compile('\W+', re.UNICODE) +rx_underscore = re.compile('_+', re.UNICODE) +def title_to_url(title, max_length = 50): + """Takes a string and makes it suitable for use in URLs""" + title = _force_unicode(title) #make sure the title is unicode + title = rx_whitespace.sub('_', title) #remove whitespace + title = rx_notsafe.sub('', title) #remove non-printables + title = rx_underscore.sub('_', title) #remove double underscores + title = title.strip('_') #remove trailing underscores + + if len(title) > max_length: + #truncate to nearest word + title = title[:max_length] + last_word = title.rfind('_') + if (last_word > 0): + title = title[:last_word] + return title diff --git a/r2/r2/models/link.py b/r2/r2/models/link.py index 1aa14e1f4..b62b7e204 100644 --- a/r2/r2/models/link.py +++ b/r2/r2/models/link.py @@ -20,7 +20,7 @@ # CondeNet, Inc. All Rights Reserved. ################################################################################ from r2.lib.db.thing import Thing, Relation, NotFound, MultiRelation -from r2.lib.utils import base_url, tup, domain, worker +from r2.lib.utils import base_url, tup, domain, worker, title_to_url from account import Account from subreddit import Subreddit from printable import Printable @@ -208,13 +208,9 @@ class Link(Thing, Printable): s = ''.join(s) return s -# @property -# def permalink(self): -# return "%s/info/%s/comments/" % (self.sr.path, self._id36) - @property def permalink(self): - return "/info/%s/comments/" % self._id36 + return "/comments/%s/%s/" % (self._id36, title_to_url(self.title)) @classmethod def add_props(cls, user, wrapped): diff --git a/r2/r2/public/static/link.js b/r2/r2/public/static/link.js index 81c79d03c..d07c987cb 100644 --- a/r2/r2/public/static/link.js +++ b/r2/r2/public/static/link.js @@ -530,9 +530,7 @@ ThingForm.prototype = { where.insertBefore(this.form, where.firstChild); } } - else { - where.insertBefore(this.form, where.firstChild); - } + where.insertBefore(this.form, where.firstChild); show(this.form); } diff --git a/r2/r2/public/static/robots.txt b/r2/r2/public/static/robots.txt index 9bc83f284..78a790d0b 100644 --- a/r2/r2/public/static/robots.txt +++ b/r2/r2/public/static/robots.txt @@ -1,5 +1,5 @@ User-Agent: * Disallow: /goto -Disallow: /*after=* -Disallow: /*before=* +Disallow: /*after= +Disallow: /*before= Allow: / diff --git a/r2/r2/templates/base.html b/r2/r2/templates/base.html index 30f4df45b..c413a8bdd 100644 --- a/r2/r2/templates/base.html +++ b/r2/r2/templates/base.html @@ -28,9 +28,13 @@ <html xmlns="http://www.w3.org/1999/xhtml" lang="${c.lang}" xml:lang="${c.lang}" ${c.lang_rtl and unsafe('dir="rtl"') or ''}> <head> -<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /> <title>${self.Title()} + + + +${self.robots()} + ##these are globals, so they should be run first