Repository navigation
Conversation
The userinfo part of AUTHORITY_PATTERN was (.+(:.*)?@)?. The outer .+ and the inner (:.*)? are both greedy and overlap, so when the authority contains no '@' the matcher explores every way of splitting the two quantifiers before failing. isValidAuthority receives the authority straight from the submitted URL, so an input like http:// followed by tens of thousands of colons drives that into quadratic work: ~32KB of colons takes tens of seconds, and it scales O(n^2). A UrlValidator is commonly attached to a public URL field, so a handful of such requests exhaust a request thread each. The inner (:.*)? is redundant: .+ already matches ':' and every other character, and the group is never read. Rewriting the userinfo as (.+@)? matches the exact same strings and captures the same host, port and extra groups, while reducing userinfo matching to a single linear scan. The host, port and extra group indices move from 3/4/5 to 2/3/4 to match. Applications relying on the old behaviour see no change: the set of accepted URLs and the parsed host and port are identical.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
UrlValidator's authority regex backtracks quadratically on an authority that contains no '@':
(.+(:.*)?@)?overlaps the outer.+with the inner(:.*)?, so a value likehttp://followed by tens of thousands of colons takes tens of seconds and scales O(n^2)isValidAuthoritygets the authority straight from the submitted URL, and a UrlValidator normally sits on a public, often pre-auth form field, so a few requests each pin a request thread(:.*)?is redundant since.+already matches:, and the group is never read, so(.+@)?accepts the identical set of URLs and parses the same host and portThe regression test submits
http://plus 50k colons and asserts it resolves well under a second; it times out against the old pattern and passes with the fix.