Crawler backend

sitescripts/crawler/web/crawler.py

-import MySQLdb, os, simplejson
+import MySQLdb, os, re, simplejson, sys
 from sitescripts.utils import cached, get_config
 from sitescripts.web import url_handler, basic_auth
 
 @cached(600)
 def _get_db():
   database = get_config().get("crawler", "database")
   dbuser = get_config().get("crawler", "dbuser")
   dbpasswd = get_config().get("crawler", "dbpassword")
   if os.name == "nt":
-    return MySQLdb.connect(user=dbuser, passwd=dbpasswd, db=database, use_unicode=True, charset="utf8", named_pipe=True)
+    return MySQLdb.connect(user=dbuser, passwd=dbpasswd, db=database,
+                           use_unicode=True, charset="utf8", named_pipe=True)
   else:
-    return MySQLdb.connect(user=dbuser, passwd=dbpasswd, db=database, use_unicode=True, charset="utf8")
+    return MySQLdb.connect(user=dbuser, passwd=dbpasswd, db=database,
+                           use_unicode=True, charset="utf8")
 
 def get_cursor():
   return _get_db().cursor(MySQLdb.cursors.DictCursor)
 
 def _fetch_crawlable_sites():
   cursor = get_cursor()
   cursor.execute("SELECT url from crawler_sites")
   results = cursor.fetchall()
   sites = [result["url"] for result in results]
   return sites
 
 @url_handler("/crawlableSites")
-@basic_auth
+@basic_auth("crawler")
 def crawlable_sites(environ, start_response):
   urls = _fetch_crawlable_sites()
   start_response("200 OK", [("Content-Type", "text/plain")])
   return "\n".join(urls)
 
 def _find_site_id(site_url):
   cursor = get_cursor()
   cursor.execute("SELECT id FROM crawler_sites WHERE url = %s", site_url)
-  return cursor.fetchall()[0]["id"]

[Wladimir Palant, 2012/09/14 17:24:18]

a) Why use fetchall() if we are only interested in one result? See fetchone().
b) What if there is no result (e.g. data consistency issue)? Please make sure
you don't fail completely then.

[Felix Dahlke, 2012/09/26 15:20:30]

Done.

+  result = cursor.fetchone()
+  return result["id"] if result else None
 
 def _read_multipart_lines(environ, line_callback):
   data_file = environ["wsgi.input"]
-  current_line = 0
+  content_type = environ.get("CONTENT_TYPE")
+  if not content_type:
+    raise ValueError("Content-Type missing from header")
 
-  while True:
-    line = data_file.readline().strip()

[Wladimir Palant, 2012/09/14 17:24:18]

Same as earlier, please use:

  for line in environ["wsgi.input"]:

Note how your loop will run forever if the boundary is not found in the input.

[Felix Dahlke, 2012/09/26 15:20:30]

Done.

-    current_line += 1
+  match = re.search(r"boundary=(.*)", content_type)
+  if not match:
+    raise ValueError("Multipart form data or boundary declaration missing")
 
-    if current_line == 1:
-      boundary = line
+  boundary = match.group(1)
+  boundary_passed = False
+  header_passed = False
+
+  for line in data_file:
+    line = line.strip()
+
+    if not boundary_passed:
+      if line == "--" + boundary:
+        boundary_passed = True
       continue
 
-    if current_line < 5 or not line:

[Wladimir Palant, 2012/09/14 17:24:18]

No, that's not how you parse multipart/form-data (classical magic number).
Please extract the boundary from environ.get('HTTP_CONTENT_DISPOSITION', '')
(should be following "; boundary=") and look for the line "--" + boundary
starting a field. It is followed by a few headers (number unknown) and an empty
line - and then the contents. "--" + boundary + "--" ends it then - assuming
that we only get one field is fine here IMHO, not the assumptions about how the
browser will format multipart/form-data.

[Felix Dahlke, 2012/09/26 15:20:30]

Done. Wow, that was one mean hack I did there :)

[Wladimir Palant, 2012/09/27 07:34:17]

Much better now :)

+    if not header_passed:
+      if not line:
+        header_passed = True
       continue
 
-    if line.startswith(boundary):
+    if line == "--" + boundary + "--":
       break
 
-    line_callback(line)
+    if line:
+      line_callback(line)
 
 def _create_run():
   cursor = get_cursor()
   cursor.execute("INSERT INTO crawler_runs () VALUES ()")
   return cursor.lastrowid
 
 def _insert_data(run_id, site, url, filtered):
   site_id = _find_site_id(site)
+  if site_id is None:
+    print >>sys.stderr, "Unable to find site '%s' in the database" % site
+    return
+
   cursor = get_cursor()
   cursor.execute("""
 INSERT INTO crawler_data (run, site, url, filtered)
 VALUES (%s, %s, %s, %s)""",
-                 (run_id, site_id, url, filtered))  
+                 (run_id, site_id, url, filtered))
 
 @url_handler("/crawlerData")
-@basic_auth
+@basic_auth("crawler")
 def crawler_data(environ, start_response):
   def line_callback(line):
-    url, site, filtered = simplejson.loads(line)
-    _insert_data(run_id, site, url, filtered)
+    try:
+      url, site, filtered = simplejson.loads(line)
+      _insert_data(run_id, site, url, filtered)
+    except simplejson.JSONDecodeError:
+      print >>sys.stderr, "Unable to parse JSON from '%s'" % line
 
   run_id = _create_run()
-  _read_multipart_lines(environ, line_callback)
-  start_response("200 OK", [("Content-Type", "text/plain")])
-  return ""
+  try:
+    _read_multipart_lines(environ, line_callback)
+    start_response("200 OK", [("Content-Type", "text/plain")])
+    return ""
+  except ValueError as e:
+    start_response("401 Bad Request", [("Content-Type", "text/plain")])
+    return e