Add stricter filename cleaning to OnDemand course parser

We've had numerous bug reports because OnDemand course parser
is leaving too many illegal characters behind. This commit adds
more calls to `clean_filename` in OnDemand parser.

fix #486
fix #481
fix #443
This commit is contained in:
Yuri Bochkarev 2016-06-14 21:20:57 +03:00
parent a2eae31f12
commit 65a2efe603
6 changed files with 56 additions and 27 deletions

View file

@ -12,7 +12,7 @@ from six import iterkeys, iteritems
from six.moves.urllib_parse import quote_plus
from .utils import (BeautifulSoup, make_coursera_absolute_url,
extend_supplement_links, clean_url)
extend_supplement_links, clean_url, clean_filename)
from .network import get_page_json
from .define import (OPENCOURSE_SUPPLEMENT_URL,
OPENCOURSE_PROGRAMMING_ASSIGNMENTS_URL,
@ -103,7 +103,7 @@ class CourseraOnDemand(object):
old-style Coursera classes. This API is by no means complete.
"""
def __init__(self, session, course_id):
def __init__(self, session, course_id, unrestricted_filenames=False):
"""
Initialize Coursera OnDemand API.
@ -112,10 +112,17 @@ class CourseraOnDemand(object):
@param course_id: Course ID from course json.
@type course_id: str
@param unrestricted_filenames: Flag that indicates whether grabbed
file names should endure stricter character filtering. @see
`clean_filename` for the details.
@type unrestricted_filenames: bool
"""
self._session = session
self._course_id = course_id
self._unrestricted_filenames = unrestricted_filenames
def extract_links_from_lecture(self,
video_id, subtitle_language='en',
resolution='540p', assets=None):
@ -209,12 +216,17 @@ class CourseraOnDemand(object):
if not images:
return
# Get assetid attribute from all images
asset_ids = [image.attrs.get('assetid') for image in images]
# Downloaded information about image assets (image IDs)
asset_list = get_page_json(self._session, OPENCOURSE_API_ASSETS_V1_URL,
id=','.join(asset_ids))
# Create a map "asset_id => asset" for easier access
asset_map = dict((asset['id'], asset) for asset in asset_list['elements'])
for image in images:
# Download each image and encode it using base64
url = asset_map[image['assetid']]['url']['url'].strip()
request = self._session.get(url)
if request.status_code == 200:
@ -264,8 +276,12 @@ class CourseraOnDemand(object):
if extension is '':
return
extension = extension.lower().strip('.').strip()
basename = os.path.basename(filename)
extension = clean_filename(
extension.lower().strip('.').strip(),
self._unrestricted_filenames)
basename = clean_filename(
os.path.basename(filename),
self._unrestricted_filenames)
url = url.strip()
if extension not in destination:
@ -611,8 +627,12 @@ class CourseraOnDemand(object):
# Build supplement links, providing nice titles along the way
for asset in asset_urls:
title = asset_tags_map[asset['id']]['name']
extension = asset_tags_map[asset['id']]['extension'].strip()
title = clean_filename(
asset_tags_map[asset['id']]['name'],
self._unrestricted_filenames)
extension = clean_filename(
asset_tags_map[asset['id']]['extension'].strip(),
self._unrestricted_filenames)
url = asset['url'].strip()
if extension not in supplement_links:
supplement_links[extension] = []
@ -654,8 +674,12 @@ class CourseraOnDemand(object):
continue
# Make lowercase and cut the leading/trailing dot
extension = extension.lower().strip('.').strip()
basename = os.path.basename(filename)
extension = clean_filename(
extension.lower().strip('.').strip(),
self._unrestricted_filenames)
basename = clean_filename(
os.path.basename(filename),
self._unrestricted_filenames)
if extension not in supplement_links:
supplement_links[extension] = []
# Putting basename into the second slot of the tuple is important

View file

@ -208,7 +208,7 @@ def get_old_style_video(session, url):
return soup.find(attrs={'type': re.compile('^video/mp4')})['src']
def parse_old_style_syllabus(session, page, reverse=False, intact_fnames=False,
def parse_old_style_syllabus(session, page, reverse=False, unrestricted_filenames=False,
subtitle_language='en'):
"""
Parse an old style Coursera course listing/syllabus page.
@ -224,7 +224,7 @@ def parse_old_style_syllabus(session, page, reverse=False, intact_fnames=False,
for stag in stags:
assert stag.contents[0] is not None, "couldn't find section"
untouched_fname = stag.contents[0].contents[1]
section_name = clean_filename(untouched_fname, intact_fnames)
section_name = clean_filename(untouched_fname, unrestricted_filenames)
logging.info(section_name)
lectures = [] # resources for 1 lecture
@ -232,7 +232,7 @@ def parse_old_style_syllabus(session, page, reverse=False, intact_fnames=False,
for vtag in stag.nextSibling.findAll('li'):
assert vtag.a.contents[0], "couldn't get lecture name"
untouched_fname = vtag.a.contents[0]
vname = clean_filename(untouched_fname, intact_fnames)
vname = clean_filename(untouched_fname, unrestricted_filenames)
logging.info(' %s', vname)
lecture = {}
lecture_page = None
@ -240,7 +240,7 @@ def parse_old_style_syllabus(session, page, reverse=False, intact_fnames=False,
for a in vtag.findAll('a'):
href = fix_url(a['href'])
untouched_fname = a.get('title', '')
title = clean_filename(untouched_fname, intact_fnames)
title = clean_filename(untouched_fname, unrestricted_filenames)
fmt = get_anchor_format(href)
if fmt in ('srt', 'txt') and subtitle_language != 'en':
title = title.replace('_en&format', '_' + subtitle_language + '&format')
@ -306,7 +306,7 @@ def parse_old_style_syllabus(session, page, reverse=False, intact_fnames=False,
return sections
def parse_on_demand_syllabus(session, page, reverse=False, intact_fnames=False,
def parse_on_demand_syllabus(session, page, reverse=False, unrestricted_filenames=False,
subtitle_language='en', video_resolution=None):
"""
Parse a Coursera on-demand course listing/syllabus page.
@ -319,7 +319,8 @@ def parse_on_demand_syllabus(session, page, reverse=False, intact_fnames=False,
'This may take some time, please be patient ...')
modules = []
json_modules = dom['courseMaterial']['elements']
course = CourseraOnDemand(session=session, course_id=dom['id'])
course = CourseraOnDemand(session=session, course_id=dom['id'],
unrestricted_filenames=unrestricted_filenames)
ondemand_material_items = OnDemandCourseMaterialItems.create(
session=session, course_name=course_name)
@ -630,6 +631,9 @@ def get_lecture_filename(combined_section_lectures_nums,
else:
lecture_filename = os.path.join(
section_dir, format_resource(lecnum + 1, lecname, title, fmt))
# Remove illegal characters
return lecture_filename
@ -648,7 +652,7 @@ def download_lectures(downloader,
combined_section_lectures_nums=False,
hooks=None,
playlist=False,
intact_fnames=False,
unrestricted_filenames=False,
ignored_formats=None,
resume=False,
skipped_urls=None,
@ -951,7 +955,7 @@ def parse_args(args=None):
help='include lecture and section name in final files')
parser.add_argument('--unrestricted-filenames',
dest='intact_fnames',
dest='unrestricted_filenames',
action='store_true',
default=False,
help='Do not limit filenames to be ASCII-only')
@ -1129,7 +1133,7 @@ def download_old_style_class(args, class_name):
# parse it
sections = parse_old_style_syllabus(session, page, args.reverse,
args.intact_fnames, subtitle_language)
args.unrestricted_filenames, subtitle_language)
downloader = get_downloader(session, class_name, args)
@ -1153,7 +1157,7 @@ def download_old_style_class(args, class_name):
args.combined_section_lectures_nums,
args.hooks,
args.playlist,
args.intact_fnames,
args.unrestricted_filenames,
ignored_formats,
args.resume,
args.video_resolution)
@ -1181,7 +1185,7 @@ def download_on_demand_class(args, class_name):
# parse it
modules = parse_on_demand_syllabus(session, page,
args.reverse,
args.intact_fnames,
args.unrestricted_filenames,
args.subtitle_language,
args.video_resolution)
@ -1217,7 +1221,7 @@ def download_on_demand_class(args, class_name):
args.combined_section_lectures_nums,
args.hooks,
args.playlist,
args.intact_fnames,
args.unrestricted_filenames,
ignored_formats,
args.resume,
None if args.disable_url_skipping else skipped_urls,

View file

@ -2,17 +2,17 @@
"pptx": [
[
"https://d396qusza40orc.cloudfront.net/learning/Powerpoints/1-1_Introduction_to_the_focused_and_diffuse_mode.pptx",
"Introduction to the Focused and Diffuse Modes"
"Introduction_to_the_Focused_and_Diffuse_Modes"
]
],
"pdf": [
[
"https://d3c33hcgiwev3.cloudfront.net/_641128be0c5ea9b054cc3008f37074fe_Introduction-to-the-Focused-and-Diffuse-Modes.pdf?Expires=1456358400&Signature=Li3AeGJVex87g4W~bPGA2uKDgTxNA~itdHcddALNuamSDDMJtmOwa4TX83MYcBlhrG2UySjTMeA0njVk8hnJh5SnbNlT7VTsOgywM54fWoOWHXGr2b1sYTLHIjQemJorMinemxnHDY2whxytvp5GFSYDExgCHmXlzvw2KGJUCSw_&Key-Pair-Id=APKAJLTNE6QMUY6HBC5A",
"Introduction to the Focused and Diffuse Modes"
"Introduction_to_the_Focused_and_Diffuse_Modes"
],
[
"https://d3c33hcgiwev3.cloudfront.net/_a0393b8c8f798d7230184f965ef5142b_Introduction-to-the-Focused-and-Diffuse-Modes-Script.pdf?Expires=1456358400&Signature=hmtU4SPPw~m8zTg7BV1Nr5MSLjIicJjvwQozu04zGAh8VF1uwfDnZUZUFUBc~8xRhtSdeoMxvxjSq5Eo4eA56KfBc9woSv~3rjFYSnMwMq9~-dC4UbaDEE74lbT68jJEOKmAOPH6cPekgfwIBgQgDEaBUfa~BgwKSAdozBrbP78_&Key-Pair-Id=APKAJLTNE6QMUY6HBC5A",
"Introduction to the Focused and Diffuse Modes Script"
"Introduction_to_the_Focused_and_Diffuse_Modes_Script"
]
]
}

View file

@ -2,11 +2,11 @@
"ipynb": [
[
"https://d396qusza40orc.cloudfront.net/phoenixassets/course1-for-students/Getting%20Started%20with%20SFrames.ipynb",
"Getting%20Started%20with%20SFrames"
"Getting_Started_with_SFrames"
],
[
"https://d396qusza40orc.cloudfront.net/phoenixassets/course1-for-students/Getting%20started%20with%20iPython%20Notebook.ipynb",
"Getting%20started%20with%20iPython%20Notebook"
"Getting_started_with_iPython_Notebook"
]
],
"html": [

View file

@ -2,7 +2,7 @@
"ipynb": [
[
"https://d396qusza40orc.cloudfront.net/phoenixassets/course1-for-students/Deep%20Features%20for%20Image%20Retrieval.ipynb",
"Deep%20Features%20for%20Image%20Retrieval"
"Deep_Features_for_Image_Retrieval"
]
],
"zip": [

View file

@ -24,7 +24,7 @@ from .define import COURSERA_URL
from six.moves import html_parser
from six import iteritems
from six.moves.urllib.parse import ParseResult
from six.moves.urllib_parse import urlparse
from six.moves.urllib_parse import unquote_plus
# six.moves doesnt support urlparse
if six.PY3: # pragma: no cover
@ -83,6 +83,7 @@ def clean_filename(s, minimal_change=False):
# First, deal with URL encoded strings
h = html_parser.HTMLParser()
s = h.unescape(s)
s = unquote_plus(s)
# Strip forbidden characters
s = (