Skip to content
This repository has been archived by the owner on Jan 25, 2024. It is now read-only.

Handle IPv6-only results even in GETHOSTBYNAME #1

Merged
merged 1 commit into from
Nov 11, 2022

Conversation

andir
Copy link

@andir andir commented Nov 8, 2022

Previously IPv6-only results would return an empty successful result
message. This doesn't seem to be a valid response from the perspective
of Glibc. In my case it caused Firefox to crash.

Previously IPv6-only results would return an empty successful result
message. This doesn't seem to be a valid response from the perspective
of Glibc. In my case it caused Firefox to crash.
@andir andir changed the title Handle IPv6 results even in GETHOSTBYNAME Handle IPv6-only results even in GETHOSTBYNAME Nov 9, 2022
@flokli
Copy link
Collaborator

flokli commented Nov 9, 2022

Previously IPv6-only results would return an empty successful result
message.

Not sure I follow. The previous code did return an error if addresses was not Ok, and you changed it to become an empty list of addresses.

We did filter out all addresses that are not IPv4 from the getaddrinfo call, so I would have assumed this to be an empty list in that case?

@grahamc
Copy link
Member

grahamc commented Nov 9, 2022

I can confirm that switching to nsncd causes firefox to crash in scenarios like this.

This PR appears to fix it. I applied it via:

  inputs.nsncd = {
    url = "github:andir/nsncd/fix-ipv6";
    flake = false;
  };

  outputs = { self, nixpkgs, nsncd, ... }: {
    nixosConfigurations.hermes-conrad = nixpkgs.lib.nixosSystem {
      system = "x86_64-linux";
      modules = [
        ({
          nixpkgs.overlays = [
            (self: super: {
              nsncd = super.nsncd.overrideAttrs (_: {
                src = nsncd;
              });
            })
          ];
        })
        ...
      

@grahamc
Copy link
Member

grahamc commented Nov 10, 2022

Not sure I follow. The previous code did return an error if addresses was not Ok, and you changed it to become an empty list of addresses.

We did filter out all addresses that are not IPv4 from the getaddrinfo call, so I would have assumed this to be an empty list in that case?

It looks to me like the change is making it return Ok(None) if there are no IPv4 addresses, and that returning an Ok(Some(...)) with an empty list is causing firefox to crash. It may be that Vec1 would be a better return type: https://crates.io/crates/vec1/1.0.0

@andir
Copy link
Author

andir commented Nov 10, 2022

Not sure I follow. The previous code did return an error if addresses was not Ok, and you changed it to become an empty list of addresses.
We did filter out all addresses that are not IPv4 from the getaddrinfo call, so I would have assumed this to be an empty list in that case?

It looks to me like the change is making it return Ok(None) if there are no IPv4 addresses, and that returning an Ok(Some(...)) with an empty list is causing firefox to crash. It may be that Vec1 would be a better return type: https://crates.io/crates/vec1/1.0.0

I fixed that in andir@16bee69 which is in another branch with tons of refactorings. Instead of requiring a Vec1 I made it so that the code is safe to use with an empty list.

@flokli
Copy link
Collaborator

flokli commented Nov 10, 2022

@andir can you cherry-pick the fixes into this PR here, and maybe improve some of the commit messages, if things are still unclear?

Happy to also merge the refactoring a, but that should probably be a separate PR :-)

@flokli
Copy link
Collaborator

flokli commented Nov 11, 2022

Alright, I'm gonna merge this and cherry-pick some of the fixes too. Thanks for the PR!

@flokli flokli merged commit 59acf07 into nix-community:host-lookups Nov 11, 2022
picnoir added a commit to picnoir/nixpkgs that referenced this pull request Nov 16, 2022
This bump fixes the response format for IPV6-only requests. The bogus
response message was crashing glibc.

See nix-community/nsncd#1 for more informations.
@andir andir deleted the fix-ipv6 branch November 17, 2022 23:31
peterhoeg pushed a commit to peterhoeg/nixpkgs that referenced this pull request Nov 18, 2022
This bump fixes the response format for IPV6-only requests. The bogus
response message was crashing glibc.

See nix-community/nsncd#1 for more informations.
rtimush pushed a commit to rtimush/nixpkgs that referenced this pull request Sep 21, 2023
This bump fixes the response format for IPV6-only requests. The bogus
response message was crashing glibc.

See nix-community/nsncd#1 for more informations.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants