Fix handling undefined length in CdxRecord

[8W9aG]

• Currently there is a bug that prevents the parsing of CdxRecord when length is "-".
• The following code has the following error:

import wayback
client = wayback.WaybackClient()
records = client.search("www.cnn.com/*", matchType="domain", filter_field="statuscode:200")
len(list(records))

wayback.exceptions.UnexpectedResponseFormat: Could not parse CDX output: "com,cnn)/ 20100218025451 http://www.cnn.com/ text/html 200 JK7O4OWOMDJHBNQJ5U43X5OQRORF6FQB -" (query: {'url': 'www.cnn.com/*', 'matchType': 'domain', 'filter': 'statuscode:200', 'showResumeKey': 'true', 'resolveRevisits': 'true'})

The exception occurs on line 542 of _client.py. The problem is the service returns "-" where length should be, probably indicating that for some reason this data is missing, however the wayback client attempts to parse it to an int. In this case, use None instead.

[Mr0grog]

Wow, I’m surprised I haven’t run into this before! Could you please add a test? After that, I’ll get this merged and released ASAP.

Since this is covering a pretty narrow scenario, it’s probably better to use a mock (most of the tests use VCR). This one is a decent example:

wayback/wayback/tests/test_client.py

Lines 166 to 190 in 024a80e

	def test_search_removes_malformed_entries(requests_mock):
	"""
	The CDX index contains many lines for things that can't actually be
	archived and will have no corresponding memento, like `mailto:` and `data:`
	URLs. We should be stripping these out.
	Because these are rare and hard to get all in a single CDX query that isn't
	*huge*, we use a made-up mock for this one instead of a VCR recording. All
	the lines in the mock file are lines from real CDX queries (we lost track
	of the specific cases that triggered that one, and it was *very* rare).
	"""
	with open(Path(__file__).parent / 'test_files' / 'malformed_cdx.txt') as f:
	bad_cdx_data = f.read()
	with WaybackClient() as client:
	requests_mock.get('http://web.archive.org/cdx/search/cdx'
	'?url=https%3A%2F%2Fepa.gov%2F%2A'
	'&from=20200418000000&to=20200419000000'
	'&showResumeKey=true&resolveRevisits=true',
	[{'status_code': 200, 'text': bad_cdx_data}])
	records = client.search('https://epa.gov/*',
	from_date=datetime(2020, 4, 18),
	to_date=datetime(2020, 4, 19))
	assert 2 == len(list(records))

But since this is such a simple case, you probably don’t need the response content in a separate file. You could just have a string right in the source code for the test.

[8W9aG]

@Mr0grog thanks for the feedback, added a test case

[Mr0grog]

Oh, thanks for catching this!

[Mr0grog]

This should probably also be asserting that the .length property on the last one is None while the others are ints, e.g:

Suggested change

	assert 5 == len(list(records))
	items = list(records)
	assert 5 == len(items)
	assert isinstance(items[0].length, int)
	assert items[4] is None

[8W9aG]

Thanks, I will make this change

[Mr0grog]

Oh, please use single quotes for non-multiline strings, too. Sorry I missed this last night.

Suggested change

	length = None if data.length == "-" else int(data.length)
	length = None if data.length == '-' else int(data.length)

[8W9aG]

Will do

[Mr0grog]

This looks great! Would you like to be listed as a contributor?

[8W9aG]

@Mr0grog That sounds great thanks!

[Mr0grog]

You can now grab this from PyPI as version 0.3.1! 🚀