Skip to content

Add ipv6 support to should_bypass_proxies - #5953

Open
derekhiggins wants to merge 1 commit into
psf:mainfrom
derekhiggins:ipv6-noproxy
Open

Add ipv6 support to should_bypass_proxies#5953
derekhiggins wants to merge 1 commit into
psf:mainfrom
derekhiggins:ipv6-noproxy

Conversation

@derekhiggins

@derekhiggins derekhiggins commented Oct 8, 2021

Copy link
Copy Markdown

Add support to should_bypass_proxies to support
IPv6 ipaddresses and CIDRs in no_proxy. Includes
adding IPv6 support to various other helper functions.

dtantsur
dtantsur approved these changes Oct 11, 2021
View reviewed changes
Comment thread requests/utils.py Outdated
derekhiggins
derekhiggins commented Oct 18, 2021
View reviewed changes
Comment thread requests/utils.py Outdated
netmask = struct.unpack('=L', socket.inet_aton(dotted_netmask(int(bits))))[0]
network = struct.unpack('=L', socket.inet_aton(netaddr))[0] & netmask
if is_ipv4_address(ip) and is_ipv4_address(netaddr):
ipaddr = struct.unpack('>L', socket.inet_aton(ip))[0]

@derekhiggins derekhiggins Oct 18, 2021

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this switch to 'big-endian' from 'native' on both big and little endian systems. Unit tests passed on both.

@derekhiggins derekhiggins mentioned this pull request Dec 21, 2022
Open
@derekhiggins

derekhiggins commented Jan 9, 2023

Copy link
Copy Markdown
Author

Hi @nateprewitt @sethmlarson , is this something you'd consider merging ?

@melewitz

melewitz commented Jan 11, 2023

Copy link
Copy Markdown

I'm facing this same issue. @nateprewitt @sethmlarson Any update on your thoughts?

@derekhiggins
derekhiggins force-pushed the ipv6-noproxy branch 2 times, most recently from f2e8dc6 to 4f89a66 Compare September 8, 2023 14:38
@novacain1

novacain1 commented Oct 11, 2023

Copy link
Copy Markdown

Running into the same problem with this, it sure would be nice to have the ability to specify IPv6 CIDRs in no_proxy. What's holding this up?

Ousret pushed a commit to jawah/niquests that referenced this pull request Oct 16, 2023
Ahmed TAHRI
🔖 Release 3.1.2
77fa29b 
**Fixed**
- Static type checker not accepting **list\[str\]** in values for argument **data**.

**Misc**
- Changed the documentation theme by **furo**.

**Added**
- IPv6 support in the `NO_PROXY` environment variable or in the **proxies** (key no_proxy) argument.
  Patch taken from idle upstream PR psf#5953
- Preemptively register a website to be HTTP/3 capable prior to the first TLS over TCP handshake.
  You can do so by doing like:

  ```python
  from niquests import Session

  s = Session()
  s.quic_cache_layer.add_domain("cloudflare.com")
  ```
- Passed **data** will be converted to form-data if headers have a Content-Type header and is set to `multipart/form-data`.
  Otherwise, by default, it is still urlencoded. If you specified a boundary, it will be used, otherwise, a random one will
  be generated.
@Ousret Ousret mentioned this pull request Oct 16, 2023
Merged
Ousret pushed a commit to jawah/niquests that referenced this pull request Oct 16, 2023
Ahmed TAHRI
🔖 Release 3.1.2
1d8561f 
**Fixed**
- Static type checker not accepting **list\[str\]** in values for argument **data**.

**Misc**
- Changed the documentation theme by **furo**.

**Added**
- IPv6 support in the `NO_PROXY` environment variable or in the **proxies** (key no_proxy) argument.
  Patch taken from idle upstream PR psf#5953
- Preemptively register a website to be HTTP/3 capable prior to the first TLS over TCP handshake.
  You can do so by doing like:

  ```python
  from niquests import Session

  s = Session()
  s.quic_cache_layer.add_domain("cloudflare.com")
  ```
- Passed **data** will be converted to form-data if headers have a Content-Type header and is set to `multipart/form-data`.
  Otherwise, by default, it is still urlencoded. If you specified a boundary, it will be used, otherwise, a random one will
  be generated.
Ousret pushed a commit to jawah/niquests that referenced this pull request Oct 16, 2023
Ahmed TAHRI
🔖 Release 3.1.2
0cc9b8a 
**Fixed**
- Static type checker not accepting **list\[str\]** in values for argument **data**.

**Misc**
- Changed the documentation theme by **furo**.

**Added**
- IPv6 support in the `NO_PROXY` environment variable or in the **proxies** (key no_proxy) argument.
  Patch taken from idle upstream PR psf#5953
- Preemptively register a website to be HTTP/3 capable prior to the first TLS over TCP handshake.
  You can do so by doing like:

  ```python
  from niquests import Session

  s = Session()
  s.quic_cache_layer.add_domain("cloudflare.com")
  ```
- Passed **data** will be converted to form-data if headers have a Content-Type header and is set to `multipart/form-data`.
  Otherwise, by default, it is still urlencoded. If you specified a boundary, it will be used, otherwise, a random one will
  be generated.
Ousret added a commit to jawah/niquests that referenced this pull request Oct 16, 2023
@Ousret
🔖 Release 3.1.2 (#37)
5cb7c7b 
**Fixed**
- Static type checker not accepting **list\[str\]** in values for argument **data**.

**Misc**
- Changed the documentation theme by **furo**.

**Added**
- IPv6 support in the `NO_PROXY` environment variable or in the **proxies** (key no_proxy) argument.
  Patch taken from idle upstream PR psf#5953
- Preemptively register a website to be HTTP/3 capable prior to the first TLS over TCP handshake.
  You can do so by doing like:

  ```python
  from niquests import Session

  s = Session()
  s.quic_cache_layer.add_domain("cloudflare.com")
  ```
- Passed **data** will be converted to form-data if headers have a Content-Type header and is set to `multipart/form-data`.
  Otherwise, by default, it is still urlencoded. If you specified a boundary, it will be used, otherwise, a random one will
  be generated.
@derekhiggins

derekhiggins commented Nov 28, 2023 •
edited
Loading

Copy link
Copy Markdown
Author

@nateprewitt @sigmavirus24 @sethmlarson would you have a chance to look at this please, its been hanging around for over 2 years and still causing me problems.

@frenzymadness

frenzymadness commented Dec 14, 2023

Copy link
Copy Markdown
Contributor

It'd be nice to have this implemented. Is there any way I can help?

@derekhiggins

derekhiggins commented Dec 14, 2023

Copy link
Copy Markdown
Author

Hi @frenzymadness , if you can review or confirm that this works for your use case then that would be appreciated but
ultimately this PR has been waiting on reviews from one of the maintainers I'm not sure how best to get it some traction besides pinging them on here.

frenzymadness
frenzymadness reviewed Dec 20, 2023
View reviewed changes

@frenzymadness frenzymadness left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the code and it looks good to me. I'm looking for more ways to test it.

Comment thread src/requests/utils.py
)
network = (network_msb << 64) ^ network_lsb
else:
return False

@frenzymadness frenzymadness Dec 20, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is returning False the correct thing to do when this function is called with IPv4 IP and IPv6 network or vice versa? What about raising an exception here?

@derekhiggins derekhiggins Jan 2, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine as is (i.e. returning false), the function is used to test if a IP address is a member of any of the CIDR's listed in no_proxy, having a no_proxy with mixed IPv4 and IPv6 CIDR's would be valid, I don't think I'd expect an exception to the raised in this instance.

@frenzymadness

frenzymadness commented Jan 2, 2024

Copy link
Copy Markdown
Contributor

We are going to test this change internally to get some confidence.

@frenzymadness

frenzymadness commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

Derek tested the package in OpenShift 4.15 and the functionality proposed here works as expected. Is there any chance to merge this?

@frenzymadness

frenzymadness commented Jan 25, 2024

Copy link
Copy Markdown
Contributor

During additional testing, my colleague raised an interesting question: Are ::/0 and 0.0.0.0/0 valid values for no_proxy? The current implementation for IPv4 does not allow masks to be lower than 1 and the new implementation for IPv6 does the same.

I see a use-case for ::/0 - let's say I have a proxy running on both protocols but I want it to be used only for IPv4 - then it might make sense to use NO_PROXY for all IPv6 addresses. What do you think?

@jadhaj jadhaj mentioned this pull request Feb 21, 2024
Closed
@frenzymadness

frenzymadness commented May 14, 2024

Copy link
Copy Markdown
Contributor

What can I do to get this merged? We've reviewed the change and tested it in a real environment with real use cases. Is there anything else we can do?

@derekhiggins
derekhiggins force-pushed the ipv6-noproxy branch 2 times, most recently from 6e0403b to bc8624b Compare May 17, 2024 15:14
@tboy1337 tboy1337 mentioned this pull request Oct 24, 2025
Closed
@derekhiggins
Add ipv6 support to should_bypass_proxies
4703994 
Add support to should_bypass_proxies to support
IPv6 ipaddresses and CIDRs in no_proxy. Includes
adding IPv6 support to various other helper functions.
@derekhiggins

derekhiggins commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Rebased onto current main.

Also worth noting - this patch has been carried in Fedora's python3-requests package since at least 2024.
It would be good to get it merged upstream rather than continued as a downstream patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@frenzymadness frenzymadness frenzymadness left review comments
@dtantsur dtantsur dtantsur approved these changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@derekhiggins @melewitz @novacain1 @frenzymadness @dtantsur