Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions newspaper/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,13 @@ def url_to_filetype(abs_url):
path = path[:-1]
path_chunks = [x for x in path.split('/') if len(x) > 0]
last_chunk = path_chunks[-1].split('.') # last chunk == file usually
file_type = last_chunk[-1] if len(last_chunk) >= 2 else None
return file_type or None
if len(last_chunk) < 2:
return None
file_type = last_chunk[-1]
# Assume that file extension is maximum 5 characters long
if len(file_type) <= 5 or file_type.lower() in ALLOWED_TYPES:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps 5 should be defined in a constants file? Ugh, but it's not like the rest of this codebase set a good example

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codelucas 5 is a quick-and-dirty heuristic, and it should be kept local. Moving it to constants would only give it an undeserved air of authority.

The proper solution is to use a extension blacklist and HEAD requests for uncertain cases.

return file_type.lower()
return None

def get_domain(abs_url, **kwargs):
"""
Expand Down
1 change: 1 addition & 0 deletions tests/data/test_urls.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@
1 http://www.alarabiya.net/ar/arab-and-world/syria/2014/02/02/%D8%A7%D9%84%D9%86%D8%A7%D8%AA%D9%88-%D9%8A%D8%B9%D9%84%D9%86-%D8%A7%D8%B3%D8%AA%D8%B9%D8%AF%D8%A7%D8%AF%D9%87-%D9%84%D8%AA%D8%AF%D9%85%D9%8A%D8%B1-%D8%A7%D9%84%D8%A3%D8%B3%D9%84%D8%AD%D8%A9-%D8%A7%D9%84%D9%83%D9%8A%D9%85%D9%8A%D8%A7%D9%88%D9%8A%D8%A9-%D8%A7%D9%84%D8%B3%D9%88%D8%B1%D9%8A%D8%A9.html
1 http://www.almasryalyoum.com/news/details/387323
1 http://tahrirnews.com/news/view.aspx?cdate=02022014&id=990c7733-9293-4441-982e-a44af000bb19
1 http://www.enfieldindependent.co.uk/news/14140506.Review_of_the_year__July/