GNOME Bugzilla – Bug 646082
Addresses from GSocket should be normalized before returning
Last modified: 2011-08-30 03:45:35 UTC
Created attachment 184570 [details] [review] gsocket: normalize ipv6 addresses before returning them I was using GSocketListener and wanted to listen on both IPv6 and IPv4. As my IPv6 socket didn't have IPV6_V6ONLY set, it was sufficient for listening on both socket families. Cool. However, that means that IPv4 connections have v6-in-v4 remote addresses which is annoying. These should be filtered out in the GSocket level so when calling _get_remote_address or _get_local_address you get an actual IPv4 address, not some kind of ::ffff:192.168.0.101. Here's a patch.
Comment on attachment 184570 [details] [review] gsocket: normalize ipv6 addresses before returning them >+ u_int32_t addr_big_endian; just "addr" is fine; we don't need to do any endian-swapping. >+ g_socket_normalize_address (&buffer); >+ > return g_socket_address_new_from_native (&buffer, len); >+ g_socket_normalize_address (&buffer); >+ > socket->priv->remote_address = g_socket_address_new_from_native (&buffer, len); seems like it would be simpler to just make g_socket_address_new_from_native() do this.
Created attachment 184689 [details] [review] gsocketaddress: normalize ipv6 addresses before parsing sockaddrs (In reply to comment #1) > seems like it would be simpler to just make g_socket_address_new_from_native() > do this. Okay, makes sense. Here's an updated patch. I could have changed more of g_socket_address_new_from_native() to use the union more instead of things like: struct sockaddr_in *addr = (struct sockaddr_in *) &(normalized.sa_in) but wanted to keep this patch small and that belongs in another patch anyway. Let me know if you want me to do that.
Comment on attachment 184689 [details] [review] gsocketaddress: normalize ipv6 addresses before parsing sockaddrs >+ /* We have to use a union here so we don't break strict aliasing >+ * rules. */ That's really ugly, and I don't get any warnings when I change it to be just a struct sockaddr_storage...
(In reply to comment #3) > That's really ugly, and I don't get any warnings when I change it to be just a > struct sockaddr_storage... Odd, I do. gsocketaddress.c: In function 'g_socket_address_new_from_native': gsocketaddress.c:212: warning: dereferencing pointer 's4' does break strict-aliasing rules gsocketaddress.c:210: warning: dereferencing pointer 's4' does break strict-aliasing rules gsocketaddress.c:205: note: initialized from here gsocketaddress.c:244: warning: dereferencing pointer 'normalized.34' does break strict-aliasing rules gsocketaddress.c:244: note: initialized from here gsocketaddress.c:259: warning: dereferencing pointer 'addr' does break strict-aliasing rules gsocketaddress.c:251: note: initialized from here gsocketaddress.c:274: warning: dereferencing pointer 'addr' does break strict-aliasing rules gsocketaddress.c:266: note: initialized from here gsocketaddress.c:290: warning: dereferencing pointer 'addr' does break strict-aliasing rules gsocketaddress.c:282: note: initialized from here That's with simply getting rid of the union and using "struct sockaddr_storage normalized" directly.
(In reply to comment #4) > (In reply to comment #3) > > That's really ugly, and I don't get any warnings when I change it to be just a > > struct sockaddr_storage... > > Odd, I do. very belatedly... it turned out that this was lossage caused by ccache; I wasn't really using the compiler options I thought I was.
Comment on attachment 184689 [details] [review] gsocketaddress: normalize ipv6 addresses before parsing sockaddrs so, the normalization thing really only applies to IPv6 addresses, so it would be simpler to just have it exist entirely within the G_SOCKET_FAMILY_IPV6 branch of g_socket_address_new_from_native(), rather than "normalizing" IPv4 and UNIX sockaddrs to themselves. Also, I just now pushed some additions to gio/tests/socket.c to make it do more testing that GSocket works, and it would be good to add another test there for this case too. Eg, that if you create an IPv6 server, and g_socket_speaks_ipv4() returns TRUE on it, and you connect to it via an IPv4 address, that the right thing happens.
Created attachment 194945 [details] [review] GSocketAddress: create IPv4 addresses for IPv6 mapped-IPv4 addresses based on a patch from Jonny Lamb ==== So, I tried this, but the test case doesn't work. I can't seem to create a socket that listens on both ipv6 and ipv4... what am I doing wrong?
figured it out; the dual v4/v6 thing only happens if you create the server socket with g_inet_address_new_any(), not g_inet_address_new_loopback(), Attachment 194945 [details] pushed as fb74f6e - GSocketAddress: create IPv4 addresses for IPv6 mapped-IPv4 addresses