From 65a2efe6038498058c262ef6f8f2e82cebbdd5e3 Mon Sep 17 00:00:00 2001 From: Yuri Bochkarev Date: Tue, 14 Jun 2016 21:20:57 +0300 Subject: [PATCH] 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 --- coursera/api.py | 40 +++++++++++++++---- coursera/coursera_dl.py | 28 +++++++------ ...-links-from-lectures-url-asset-output.json | 6 +-- .../fixtures/json/supplement-skip-sites.json | 4 +- .../fixtures/json/supplement-two-zips.json | 2 +- coursera/utils.py | 3 +- 6 files changed, 56 insertions(+), 27 deletions(-) diff --git a/coursera/api.py b/coursera/api.py index 6cc5f87..c54e60c 100644 --- a/coursera/api.py +++ b/coursera/api.py @@ -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 diff --git a/coursera/coursera_dl.py b/coursera/coursera_dl.py index 1aace79..2e2631b 100644 --- a/coursera/coursera_dl.py +++ b/coursera/coursera_dl.py @@ -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, diff --git a/coursera/test/fixtures/json/supplement-extract-links-from-lectures-url-asset-output.json b/coursera/test/fixtures/json/supplement-extract-links-from-lectures-url-asset-output.json index 28d23d3..4fc38fe 100644 --- a/coursera/test/fixtures/json/supplement-extract-links-from-lectures-url-asset-output.json +++ b/coursera/test/fixtures/json/supplement-extract-links-from-lectures-url-asset-output.json @@ -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" ] ] } diff --git a/coursera/test/fixtures/json/supplement-skip-sites.json b/coursera/test/fixtures/json/supplement-skip-sites.json index 52bddfb..03d6c41 100644 --- a/coursera/test/fixtures/json/supplement-skip-sites.json +++ b/coursera/test/fixtures/json/supplement-skip-sites.json @@ -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": [ diff --git a/coursera/test/fixtures/json/supplement-two-zips.json b/coursera/test/fixtures/json/supplement-two-zips.json index 39cb087..664f21f 100644 --- a/coursera/test/fixtures/json/supplement-two-zips.json +++ b/coursera/test/fixtures/json/supplement-two-zips.json @@ -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": [ diff --git a/coursera/utils.py b/coursera/utils.py index 6b66fcc..60e9425 100644 --- a/coursera/utils.py +++ b/coursera/utils.py @@ -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 doesn’t 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 = (