Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace manual host parsing code with parse-hosts crate #15756

Closed
jdm opened this issue Feb 27, 2017 · 3 comments
Closed

Replace manual host parsing code with parse-hosts crate #15756

jdm opened this issue Feb 27, 2017 · 3 comments
Assignees
Labels

Comments

@jdm
Copy link
Member

@jdm jdm commented Feb 27, 2017

https://crates.io/crates/parse-hosts
https://dxr.mozilla.org/servo/source/components/net_traits/hosts.rs

I haven't looked into the crate's API yet, but it could be a straightforward replacement.

@ferjm This might be a nice second issue to work on.

@jdm jdm added the A-network label Feb 27, 2017
@emilio
Copy link
Member

@emilio emilio commented Feb 28, 2017

For the record: license of that crate is listed as "unstandard", but looking at the repo it seems "CC0", which I think it's fine.

@ferjm ferjm self-assigned this Feb 28, 2017
@jdm
Copy link
Member Author

@jdm jdm commented Feb 28, 2017

I would like @avadacatavra to review the future PR that fixes this.

@ferjm
Copy link
Member

@ferjm ferjm commented Mar 1, 2017

Thank you @emilio, I agree that CC0 should be fine.

bors-servo added a commit that referenced this issue Mar 9, 2017
Replace manual host parsing code with parse-host crate

This patch is replacing the code to parse the hosts file with the [parse-hosts](https://crates.io/crates/parse-hosts) crate. This crate has a [CC0 1.0 Universal License](https://creativecommons.org/publicdomain/zero/1.0/deed.en).

I could have used [HostsFile::load()](https://clarcharr.github.io/parse-hosts/parse_hosts/struct.HostsFile.html#method.load) directly, but this method loads `/etc/hosts` by default and does not allow to override the default path (for example with [env::var("HOST_FILE")](https://dxr.mozilla.org/servo/source/components/net_traits/hosts.rs#19)), so I kept the existing code to open and read the content of `env::var("HOST_FILE")` and also kept the [parse_hostsfile](https://dxr.mozilla.org/servo/source/components/net_traits/hosts.rs#42) method (used by the unit tests), but I modified it to use [HostsFile::read_buffered](https://clarcharr.github.io/parse-hosts/parse_hosts/struct.HostsFile.html#method.read_buffered), which is doing the actual hosts parsing for a given string buffer.

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #15756 .
- [X] There are tests for these changes (tests/unit/net/resource_thread.rs and tests/unit/net/http_loader.rs)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15783)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Mar 10, 2017
Replace manual host parsing code with parse-host crate

This patch is replacing the code to parse the hosts file with the [parse-hosts](https://crates.io/crates/parse-hosts) crate. This crate has a [CC0 1.0 Universal License](https://creativecommons.org/publicdomain/zero/1.0/deed.en).

I could have used [HostsFile::load()](https://clarcharr.github.io/parse-hosts/parse_hosts/struct.HostsFile.html#method.load) directly, but this method loads `/etc/hosts` by default and does not allow to override the default path (for example with [env::var("HOST_FILE")](https://dxr.mozilla.org/servo/source/components/net_traits/hosts.rs#19)), so I kept the existing code to open and read the content of `env::var("HOST_FILE")` and also kept the [parse_hostsfile](https://dxr.mozilla.org/servo/source/components/net_traits/hosts.rs#42) method (used by the unit tests), but I modified it to use [HostsFile::read_buffered](https://clarcharr.github.io/parse-hosts/parse_hosts/struct.HostsFile.html#method.read_buffered), which is doing the actual hosts parsing for a given string buffer.

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #15756 .
- [X] There are tests for these changes (tests/unit/net/resource_thread.rs and tests/unit/net/http_loader.rs)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15783)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.