Skip to content

Conversation

@imjoehaines
Copy link
Contributor

@imjoehaines imjoehaines commented May 4, 2020

Goal

Webob will raise exceptions if given a URL it cannot decode. This is probably a good thing for applications, but we should be more resilient

We already have a helper for getting the path of a URL which handles bad encoding using urllib

Linked issues

Fixes #185

Review

For the submitter, initial self-review:

  • Commented on code changes inline explain the reasoning behind the approach
  • Reviewed the test cases added for completeness and possible points for discussion
  • A changelog entry was added for the goal of this pull request
  • Check the scope of the changeset - is everything in the diff required for the pull request?
  • This pull request is ready for:
    • Initial review of the intended approach, not yet feature complete
    • Structural review of the classes, functions, and properties modified
    • Final review

For the pull request reviewer(s), this changeset has been reviewed for:

  • Consistency across platforms for structures or concepts added or modified
  • Consistency between the changeset and the goal stated above
  • Internal consistency with the rest of the library - is there any overlap between existing interfaces and any which have been added?
  • Usage friction - is the proposed change in usage cumbersome or complicated?
  • Performance and complexity - are there any cases of unexpected O(n^3) when iterating, recursing, flat mapping, etc?
  • Concurrency concerns - if components are accessed asynchronously, what issues will arise
  • Thoroughness of added tests and any missing edge cases
  • Idiomatic use of the language

@imjoehaines imjoehaines changed the base branch from master to next May 5, 2020 08:13
@imjoehaines imjoehaines changed the base branch from next to master May 5, 2020 08:15
@imjoehaines imjoehaines changed the base branch from master to next May 5, 2020 08:15
@imjoehaines imjoehaines marked this pull request as ready for review May 5, 2020 08:25
@imjoehaines imjoehaines requested a review from tomlongridge May 5, 2020 08:25
Webob will raise exceptions if given a URL it cannot decode. This is
probably a good thing for applications, but we should be more
resilient

We already have a helper for getting the path of a URL which handles
bad encoding, so we can fix this issue quite easily

See #185
@imjoehaines imjoehaines force-pushed the dont-raise-on-wrongly-encoded-urls branch from 302a267 to a8893ca Compare May 5, 2020 09:08
@imjoehaines imjoehaines removed the request for review from tomlongridge May 5, 2020 09:23
@imjoehaines imjoehaines marked this pull request as draft May 5, 2020 09:23
This isn't present on Python 2
@imjoehaines imjoehaines marked this pull request as ready for review May 5, 2020 10:34
@imjoehaines imjoehaines merged commit c190653 into next May 5, 2020
@imjoehaines imjoehaines deleted the dont-raise-on-wrongly-encoded-urls branch May 5, 2020 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WSGI middleware breaks when handling badly encoded URLs

3 participants