Skip to content
This repository has been archived by the owner on Nov 15, 2019. It is now read-only.

Bootstrap file's hardcoded endpoints section is erased #174

Closed
Fraser999 opened this issue Jun 26, 2015 · 10 comments
Closed

Bootstrap file's hardcoded endpoints section is erased #174

Fraser999 opened this issue Jun 26, 2015 · 10 comments
Assignees

Comments

@Fraser999
Copy link
Contributor

I ran a Client example with a bootstrap file containing the following at the start:

{
  "preferred_port": {
    "variant": "Tcp",
    "fields": [
      5483
    ]
  },
  "hard_coded_contacts": [
    {
      "endpoint": "45.55.207.180:5483"
    },
    {
      "endpoint": "178.62.7.96:5483"
    },
    {
      "endpoint": "128.199.199.210:5483"
    },
    {
      "endpoint": "37.59.98.1:5483"
    },
    {
      "endpoint": "45.79.93.11:5483"
    },
    {
      "endpoint": "45.79.2.52:5483"
    },
  ],
  "contacts": [
    {
      "endpoint": "45.55.207.180:5483"
    },
    {
      "endpoint": "178.62.7.96:5483"
    },
    {
      "endpoint": "128.199.199.210:5483"
    },
    {
      "endpoint": "37.59.98.1:5483"
    },
    {
      "endpoint": "45.79.93.11:5483"
    },
    {
      "endpoint": "45.79.2.52:5483"
    },
  ]
}

After the example ran, it contained:

{
  "preferred_port": {
    "variant": "Tcp",
    "fields": [
      0
    ]
  },
  "hard_coded_contacts": [],
  "contacts": [
    {
      "endpoint": "127.0.0.1:48660"
    },
    {
      "endpoint": "192.168.1.65:48660"
    },
    {
      "endpoint": "[::1]:48660"
    }
  ]
}
@dirvine
Copy link
Contributor

dirvine commented Jun 26, 2015

This certainly requires a unit test in that rs file for sure.

@chandraprakash
Copy link
Contributor

Looks like the code is doing the right thing, overwriting the cache file if it can't read/parse it.
An extra , after the last element should not be provided (it fails parse).

Just connected crust_peer to droplets using the same file (removed , after last element of vector)

prakash@machine:~/rust/crust (master)$ cargo run --example crust_peer
     Running `target/debug/examples/crust_peer`
Listening for new connections on Tcp(5483), and listening for UDP broadcast on port 9999.
Bootstrapped to Tcp(V4(178.62.7.96:5483))

Connected to peer at Tcp(V4(178.62.7.96:5483))
1 connected node: Tcp(V4(178.62.7.96:5483))
Enter command (stop | connect <endpoint> | send <endpoint> <message>)>
Received from Tcp(V4(178.62.7.96:5483)) message: non-UTF-8 message of 6 bytes
Enter command (stop | connect <endpoint> | send <endpoint> <message>)>
Connected to peer at Tcp(V4(45.55.207.180:5483))
2 connected nodes: Tcp(V4(178.62.7.96:5483)) Tcp(V4(45.55.207.180:5483))
Enter command (stop | connect <endpoint> | send <endpoint> <message>)>
Enter command (stop | connect <endpoint> | send <endpoint> <message>)>
Received from Tcp(V4(45.55.207.180:5483)) message: non-UTF-8 message of 6 bytes
Enter command (stop | connect <endpoint> | send <endpoint> <message>)>

@chandraprakash
Copy link
Contributor

I will add a log statement to say its fails to read the file. Also there is a test already to write and read the file.

@chandraprakash
Copy link
Contributor

Valid bootstrap cache file

{
  "preferred_port": {
    "variant": "Tcp",
    "fields": [
      5483
    ]
  },
  "hard_coded_contacts": [
    {
      "endpoint": "45.55.207.180:5483"
    },
    {
      "endpoint": "178.62.7.96:5483"
    },
    {
      "endpoint": "128.199.199.210:5483"
    },
    {
      "endpoint": "37.59.98.1:5483"
    },
    {
      "endpoint": "45.79.93.11:5483"
    },
    {
      "endpoint": "45.79.2.52:5483"
    }
  ],
  "contacts": [
    {
      "endpoint": "45.55.207.180:5483"
    },
    {
      "endpoint": "178.62.7.96:5483"
    },
    {
      "endpoint": "128.199.199.210:5483"
    },
    {
      "endpoint": "37.59.98.1:5483"
    },
    {
      "endpoint": "45.79.93.11:5483"
    },
    {
      "endpoint": "45.79.2.52:5483"
    }
  ]
}

@dirvine
Copy link
Contributor

dirvine commented Jun 27, 2015

I have re-opend this issue as I think the app should exit with a message stating the file is invalid, allowing the user to fix or delete it. The return code should indicate invalid boostrap-cache file

@Fraser999
Copy link
Contributor Author

Are you talking about Crust invoking something like std::process::exit? If so, it seems a bit ferocious to me! I guess the issue is that the bootstrap process could still succeed via the beacon despite the bootstrap file being corrupted.

I suppose for our normal use (i.e. where we're not tinkering around with the file for testing purposes) it would be unlikely that the file would be corrupted, and such a problem is maybe serious enough to warrant termination. But I'm not sure if that should be forced on all users of Crust.

Maybe a middle ground of abandoning the bootstrap process by returning an Err would be best?

@dirvine
Copy link
Contributor

dirvine commented Jun 28, 2015

Yes it's normal for a process with a config file to abort on file read error. Apache, ftp servers, mail servers etc. all would do that as it means the user may have been trying to set something and did it wrong. So I think it's an expected thing to happen. Personally I would go for exit with a negative return code as we cannot tell if a user is trying to alter the file or if it's actually corrupted, or they have just made a mistake. We could add an option to start again with over-write etc. but then again maybe that's just taking it to far.

@dirvine
Copy link
Contributor

dirvine commented Jun 28, 2015

It may be the file needs split, a config part where we would exit and purely cache part where we would over-write.

@chandraprakash
Copy link
Contributor

+1 for the split between config and cache. I will detail a task to address this.

@chandraprakash
Copy link
Contributor

Added JIRA task to address this issue.
https://maidsafe.atlassian.net/browse/MAID-1149

@ned14 ned14 closed this as completed in 5ed4ca9 Jul 13, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants