idna bypass

This addresses #3687 allowing URLs that are already idna encoded to pass through while everything else is still properly encoded.

Alright, all systems are nominal. 🚀


Comment updated, hopefully that's a bit closer.


Alright, ASCII check updated 😃 


OK, we're all moved over to _internal_utils.


> An alternative solution to this is to say that if the idna module fails to encode we'll just go ahead and try to encode as ASCII. If that works then we shrug our shoulders and say everything is probably ok, and if it fails we catch that and throw InvalidURL. @sigmavirus24, how does that sound?

([comment](/kennethreitz/requests/issues/3683#issuecomment-261240279))

Do we want to hold here until a decision is made on this so we're not having to back things out?


It's certainly worth us holding off until we can resolve that ambiguity.


That method sounds good to me. I'm not as familiar with the idna standard though, so while it sounds perfectly practical, I don't know if it fits in with the rest of the work around the standard.


Ok then, let's go ahead with that and see how it goes.


I guess we should probably clarify what we're attempting to do here. The above comment was in reference to the the inability to use IPv6 with Requests 2.12+. Personally, I think that definitely needs to be addressed, the other complaints I have no opinion on.

The fallback approach though essentially gives carte blanche to anything that can be ASCII encoded, which seems to defeat some benefits of doing IDNA encoding. They've special cased `.` delimiters in the IDNA library, I'm assuming for things like subdomains, which allows IPv4 addresses to pass through (unintentionally?). However, they don't appear to have taken into account IPv6 style addresses with `:` delimiters or wrapping brackets.

From a cursory reading of [RFC5891](https://tools.ietf.org/html/rfc5891), it seems like IDNA is only intended for domain names. That would suggest to me we shouldn't be passing IP addresses into this function to begin with.

If we're ONLY attempting to handle IPv6, we can check to skip `host` values that are entirely numeric or delimiters. Otherwise, if we're entertaining allowing things like underscores then maybe the passthrough is the only viable option to avoid constantly programming around use cases.

**Edit**: Sorry @Lukasa I just realized I essentially reiterated your entire thought process from the other thread. Vendoring in a dependency is likely not what we want to do. `any([c.isalpha() for c in host])` is probably kludgy enough to keep out of the main path for Requests. So I guess that leaves the bypass.


Yup, that's why I landed on the bypass: it just seems like the simplest approach that is also in line with what most browsers do (i.e. try to do the right thing but allow the wrong thing if it's unambiguous). In this case I'd like to filter out certain obviously bad URLs that IDNA was saving us from (those with leading wildcards, for example), but otherwise if they're the kind of garden-variety bad that issue #3683 is discussing (or if they aren't hostnames at all!) then we can probably just pass them through and let the rest of the codebase handle them.


Ok, I removed the initial check for `xn--` because we end up performing fairly redundant logic both before the try...except block and again in the except block itself.

This should work for our given test cases, as well as the other concerns about unix socket files and hostnames containing underscores, provided they're both only ASCII. Let me know if I'm leaving anything else out.


Alright, tests extended and exception tightened.

Yep, those were oversights. Should be updated now.

Alright, I'm going to merge this now. @sigmavirus24 feel free to give feedback regardless, we can always revisit this. =) Thanks @nateprewitt!

Guys, do you have any idea of when these changes could be released?

I am tentatively considering next week for a possible release. No promises though.

Thanks for prompt response!