r108674 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r108673‎ | r108674 | r108675 >
Date:20:24, 11 January 2012
Author:aaron
Status:ok
Tags:
Comment:
* Tightened rewrite code to only apply to GET/HEAD.
* Added regex test file for rewrite.py.
* Fixed bug in regex found in tests (zone had '/').
Modified paths:
  • /trunk/extensions/SwiftMedia/wmf/rewrite.py (modified) (history)
  • /trunk/extensions/SwiftMedia/wmf/tests (added) (history)
  • /trunk/extensions/SwiftMedia/wmf/tests/rewriteRegexTest.py (added) (history)

Diff [purge]

Index: trunk/extensions/SwiftMedia/wmf/tests/rewriteRegexTest.py
——@@ -0,0 +1,74 @@
 2+import re
 3+
 4+# Rewrite URLs of these forms (source, temp, and thumbnail files):
 5+# (a) http://upload.wikimedia.org/<proj>/<lang>/.*
 6+# => http://msfe/v1/AUTH_<hash>/<proj>-<lang>-local-public/.*
 7+# (b) http://upload.wikimedia.org/<proj>/<lang>/archive/.*
 8+# => http://msfe/v1/AUTH_<hash>/<proj>-<lang>-local-public/archive.*
 9+# (c) http://upload.wikimedia.org/<proj>/<lang>/thumb/.*
 10+# => http://msfe/v1/AUTH_<hash>/<proj>-<lang>-local-thumb/.*
 11+# (d) http://upload.wikimedia.org/<proj>/<lang>/thumb/archive/.*
 12+# => http://msfe/v1/AUTH_<hash>/<proj>-<lang>-local-thumb/archive/.*
 13+# (e) http://upload.wikimedia.org/<proj>/<lang>/thumb/temp/.*
 14+# => http://msfe/v1/AUTH_<hash>/<proj>-<lang>-local-thumb/temp/.*
 15+# (f) http://upload.wikimedia.org/<proj>/<lang>/temp/.*
 16+# => http://msfe/v1/AUTH_<hash>/<proj>-<lang>-local-temp/.*
 17+
 18+# The regex to test (for rewrite.py)
 19+regex = r'^/(?P<proj>[^/]+)/(?P<lang>[^/]+)/((?P<zone>thumb|temp)/)?(?P<path>((temp|archive)/)?[0-9a-f]/(?P<shard>[0-9a-f]{2})/.+)$'
 20+
 21+# [url,proj,lang,zone,shard,path]
 22+cases = []
 23+cases.append( ['/wikipedia/commons/a/ab/file.jpg',
 24+ 'wikipedia', 'commons', None, 'ab', 'a/ab/file.jpg'] )
 25+cases.append( ['/wikipedia/commons/archive/a/ab/file.jpg',
 26+ 'wikipedia', 'commons', None, 'ab', 'archive/a/ab/file.jpg'] )
 27+cases.append( ['/wikipedia/commons/thumb/a/ab/file.jpg',
 28+ 'wikipedia', 'commons', 'thumb', 'ab', 'a/ab/file.jpg'] )
 29+cases.append( ['/wikipedia/commons/thumb/archive/a/ab/file.jpg',
 30+ 'wikipedia', 'commons', 'thumb', 'ab', 'archive/a/ab/file.jpg'] )
 31+cases.append( ['/wikipedia/commons/thumb/temp/a/ab/file.jpg',
 32+ 'wikipedia', 'commons', 'thumb', 'ab', 'temp/a/ab/file.jpg'] )
 33+cases.append( ['/wikipedia/commons/temp/a/ab/file.jpg',
 34+ 'wikipedia', 'commons', 'temp', 'ab', 'a/ab/file.jpg'] )
 35+
 36+# Tests for valid URLs
 37+print "Testing valid URLs..."
 38+for case in cases:
 39+ url = case[0]
 40+ proj = case[1]
 41+ lang = case[2]
 42+ zone = case[3]
 43+ shard = case[4]
 44+ path = case[5]
 45+
 46+ match = re.match(regex, url)
 47+ if match:
 48+ if match.group('proj') != proj:
 49+ print "FAILED for url %s; expected %s but got %s" % (url,proj,match.group('proj'))
 50+ elif match.group('lang') != lang:
 51+ print "FAILED for url %s; expected %s but got %s" % (url,lang,match.group('lang'))
 52+ elif match.group('zone') != zone:
 53+ print "FAILED for url %s; expected %s but got %s" % (url,zone,match.group('zone'))
 54+ elif match.group('shard') != shard:
 55+ print "FAILED for url %s; expected %s but got %s" % (url,shard,match.group('shard'))
 56+ elif match.group('path') != path:
 57+ print "FAILED for url %s; expected %s but got %s" % (url,path,match.group('path'))
 58+ else:
 59+ print "PASSED for url %s" % url
 60+ else:
 61+ print "FAILED; no match for url %s" % url
 62+
 63+# Tests for invalid URLs
 64+print "\nTesting invalid URLs..."
 65+cases = []
 66+cases.append( '/wikipedia/commons/deleted/a/ab/file.jpg' ); # private
 67+cases.append( '/wikipedia/commons/thumb' ); # no file
 68+cases.append( '/wikipedia/commons/thumb/a/' ); # no file
 69+cases.append( '/wikipedia/commons/thumb/a/ab/' ); # no file
 70+
 71+for url in cases:
 72+ if re.match(regex, url):
 73+ print "FAILED; should not match url %s" % url
 74+ else:
 75+ print "PASSED for url %s" % url
\ No newline at end of file
Index: trunk/extensions/SwiftMedia/wmf/rewrite.py
——@@ -157,15 +157,15 @@
158158 def __call__(self, env, start_response):
159159 #try: commented-out while debugging so you can see where stuff happened.
160160 req = webob.Request(env)
161 - # PUT requests never need rewriting.
162 - if req.method == 'PUT':
 161+ # End-users should only do GET/HEAD, nothing else needs a rewrite
 162+ if req.method != 'GET' and req.method != 'HEAD':
163163 return self.app(env, start_response)
164164
165 - # double (or triple, etc.) slashes in the URL should be ignored; collapse them. fixes bug 32864
 165+ # Double (or triple, etc.) slashes in the URL should be ignored; collapse them. fixes bug 32864
166166 while(req.path_info != req.path_info.replace('//', '/')):
167167 req.path_info = req.path_info.replace('//', '/')
168168
169 - # if it already has AUTH, presume that it's good. #07. fixes bug 33620
 169+ # If it already has AUTH, presume that it's good. #07. fixes bug 33620
170170 hasauth = re.search('/AUTH_[0-9a-fA-F]{32}/', req.path)
171171 if req.path.startswith('/auth') or hasauth:
172172 return self.app(env, start_response)
——@@ -186,7 +186,7 @@
187187 # => http://msfe/v1/AUTH_<hash>/<proj>-<lang>-local-thumb/temp/.*
188188 # (f) http://upload.wikimedia.org/<proj>/<lang>/temp/.*
189189 # => http://msfe/v1/AUTH_<hash>/<proj>-<lang>-local-temp/.*
190 - match = re.match(r'^/(?P<proj>[^/]+)/(?P<lang>[^/]+)/(?P<zone>(thumb|temp)/)?(?P<path>((temp|archive)/)?[0-9a-f]/(?P<shard>[0-9a-f]{2})/.+)$', req.path)
 190+ match = re.match(r'^/(?P<proj>[^/]+)/(?P<lang>[^/]+)/((?P<zone>thumb|temp)/)?(?P<path>((temp|archive)/)?[0-9a-f]/(?P<shard>[0-9a-f]{2})/.+)$', req.path)
191191 if match:
192192 # Get the repo zone (if not provided that means "public")
193193 zone = match.group('zone') if match.group('zone') else 'public'

Status & tagging log