Strip extraneous characters at end of URLs

[cwdavies]

Update handle_404_error to remove the last character from the URL if that character is in the extraneous_char_list

Additions

• test_handle_404_error_strip_extraneous_chars

Removals

• URLs that end in %20) have these two characters removed

Testing

1. tox -e unittest-current cfgov.tests.test_urls.HandleErrorTestCase

Checklist

• PR has an informative and human-readable title
• Changes are limited to a single goal (no scope creep)
• Code can be automatically merged (no conflicts)
• Code follows the standards laid out in the CFPB development guidelines
• Passes all existing automated tests
• Any change in functionality is tested
• New functions are documented (with a description, list of inputs, and expected output)
• Reviewers requested with the Reviewers tool ➡️

[willbarton]

Do you think we could do this with a regular expression matching any of those characters multiple times at the end of a string and then using re.sub() on the request.path? That would generalize this to more than just the last 1-2 characters of the string.

Something like this (pseudocode, I haven't tried it):

extraneous_char_re = re.compile(r'[!#$%&()*+,-.:;<=>?@\[\]^_`{|}~]+$')
request.path = re.sub(extraneous_char_re, '', request.path)

[Scotchester]

Do you think we could do this with a regular expression matching any of those characters multiple times at the end of a string and then using re.sub() on the request.path? That would generalize this to more than just the last 1-2 characters of the string.

Something like this (pseudocode, I haven't tried it):

extraneous_char_re = re.compile(r'[!#$%&()*+,-.:;<=>?@\[\]^_`{|}~]+$')
request.path = re.sub(extraneous_char_re, '', request.path)

I had a similar thought, Will. Or if we wanted to avoid regex, we could while through basic string comparisons of the final character until that final character is not one of the extraneous characters.

I was testing something to that effect yesterday afternoon, but ran into some weirdness with how browsers convert spaces to %20 that I couldn't resolve before the end of the day.

[willbarton]

I haven't tested, but I'd assume that re.sub is more performant than a while loop over the path.

[Scotchester]

I haven't tested, but I'd assume that re.sub is more performant than a while loop over the path.

Probably true.

[cwdavies]

This refactoring of handle_404_error looks great as it redirects to a lowercase path and strips off extraneous characters from the end. A unit test should be added that has a mixed case URL with one or two characters from extraneous_char_re at the end of the URL.

[Scotchester]

@cwdavies and @willbarton Retreating to just doing a single redirect if the path changed at all. Reasons are detailed in the 8b495e0 commit message. Let me know if you have questions or concerns.

[willbarton]

@Scotchester nope, that sounds reasonable.

[Scotchester]

(Note: Still need to write more tests before merging.)

[Scotchester]

New commit pushed with what I think are adequate tests. Ready for final review, @cwdavies @chosak @willbarton @higs4281.

[cwdavies]

@Scotchester has provided really good unit tests that check one, two, and multiple extraneous characters at end of URL in addition to mixed case for the new handle_404_error feature.

Requested change was made, and subsequently refactored away