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

Support parsing IP addresses from a byte string #94890

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

marmeladema
Copy link
Contributor

@marmeladema marmeladema commented Mar 12, 2022

Fixes #94821

The goal is to be able to parse addresses from a byte string without requiring to do any utf8 validation. Since internally the parser already works on byte strings, this should be possible and I personally already needed this in the past too.

I used the proposed approach from the issue by implementing TryFrom<&'a [u8]> for all 6 address types (3 ip address types and 3 socket address types). I believe implementing stable traits for stable types is insta-stable so this will probably need an FCP?

Switched to an unstable inherent method approach called parse_ascii as requested.

cc @jyn514

@rust-highfive
Copy link
Collaborator

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 12, 2022
@nagisa
Copy link
Member

nagisa commented Mar 12, 2022

I don't believe TryFrom is a correct approach, given the gratuitous amount of confusion about what the referenced issue was about. I'd say an inherent function along the lines of IpAddr::parse_ascii would likely be unambiguous in what it means.

@marmeladema marmeladema force-pushed the ip-addr-try-from-bytes branch from c4bffc0 to 85308fd Compare March 13, 2022 00:12
@marmeladema
Copy link
Contributor Author

Alright, I think I agree it's clearer to have an inherent method for this. I have updated the code and left the tracking issue empty for now in the unstable directives. This PR now only adds unstable methods so it might not require an FCP at this stage right?
I'll create the tracking issue once the code here is approved 👍

@marmeladema marmeladema force-pushed the ip-addr-try-from-bytes branch from 85308fd to 33b6772 Compare March 13, 2022 08:53
@yaahc yaahc added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 18, 2022
@marmeladema
Copy link
Contributor Author

@yaahc @nagisa any interest in this or should the associated issue be rejected and this PR closed?

@yaahc
Copy link
Member

yaahc commented Apr 8, 2022

I haven't had a chance to review this yet but based on the description I'm assuming there will be interest in these APIs. Also, please forgive me if I'm reading too much into your statement but if you're asking because you are wondering why there hasn't been a review that's just because t-libs-api review can take quite a while1, but one of us will get to this review as soon as we're able.

Footnotes

  1. My PR which I pushed in october which only just got approved today: https://github.com/rust-lang/rust/pull/90066

@bors
Copy link
Contributor

bors commented Apr 20, 2022

☔ The latest upstream changes (presumably #96253) made this pull request unmergeable. Please resolve the merge conflicts.

@yaahc
Copy link
Member

yaahc commented Apr 29, 2022

I'm reassigning this PR because I'm taking a break from the review rotation for a little while. Thank you for your patience.

r? rust-lang/libs-api

@rust-highfive rust-highfive assigned m-ou-se and unassigned yaahc Apr 29, 2022
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 20, 2022
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2022
@joshtriplett
Copy link
Member

joshtriplett commented Aug 26, 2022

This seems reasonable to me.

r=me after you fix the merge conflicts and add a tracking issue.

@bors delegate+

@bors
Copy link
Contributor

bors commented Aug 26, 2022

✌️ @marmeladema can now approve this pull request

@marmeladema marmeladema force-pushed the ip-addr-try-from-bytes branch from 129cc05 to 8bb4b5f Compare August 26, 2022 12:17
@marmeladema
Copy link
Contributor Author

@bors r=joshtriplett

@bors
Copy link
Contributor

bors commented Aug 26, 2022

📌 Commit 8bb4b5f has been approved by joshtriplett

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 26, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 26, 2022
… r=joshtriplett

Support parsing IP addresses from a byte string

Fixes rust-lang#94821

The goal is to be able to parse addresses from a byte string without requiring to do any utf8 validation. Since internally the parser already works on byte strings, this should be possible and I personally already needed this in the past too.

~~I used the proposed approach from the issue by implementing `TryFrom<&'a [u8]>` for all 6 address types (3 ip address types and 3 socket address types). I believe implementing stable traits for stable types is insta-stable so this will probably need an FCP?~~

Switched to an unstable inherent method approach called `parse_ascii` as requested.

cc `@jyn514`
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Aug 27, 2022
… r=joshtriplett

Support parsing IP addresses from a byte string

Fixes rust-lang#94821

The goal is to be able to parse addresses from a byte string without requiring to do any utf8 validation. Since internally the parser already works on byte strings, this should be possible and I personally already needed this in the past too.

~~I used the proposed approach from the issue by implementing `TryFrom<&'a [u8]>` for all 6 address types (3 ip address types and 3 socket address types). I believe implementing stable traits for stable types is insta-stable so this will probably need an FCP?~~

Switched to an unstable inherent method approach called `parse_ascii` as requested.

cc ``@jyn514``
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 28, 2022
… r=joshtriplett

Support parsing IP addresses from a byte string

Fixes rust-lang#94821

The goal is to be able to parse addresses from a byte string without requiring to do any utf8 validation. Since internally the parser already works on byte strings, this should be possible and I personally already needed this in the past too.

~~I used the proposed approach from the issue by implementing `TryFrom<&'a [u8]>` for all 6 address types (3 ip address types and 3 socket address types). I believe implementing stable traits for stable types is insta-stable so this will probably need an FCP?~~

Switched to an unstable inherent method approach called `parse_ascii` as requested.

cc `@jyn514`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 28, 2022
… r=joshtriplett

Support parsing IP addresses from a byte string

Fixes rust-lang#94821

The goal is to be able to parse addresses from a byte string without requiring to do any utf8 validation. Since internally the parser already works on byte strings, this should be possible and I personally already needed this in the past too.

~~I used the proposed approach from the issue by implementing `TryFrom<&'a [u8]>` for all 6 address types (3 ip address types and 3 socket address types). I believe implementing stable traits for stable types is insta-stable so this will probably need an FCP?~~

Switched to an unstable inherent method approach called `parse_ascii` as requested.

cc ``@jyn514``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 28, 2022
… r=joshtriplett

Support parsing IP addresses from a byte string

Fixes rust-lang#94821

The goal is to be able to parse addresses from a byte string without requiring to do any utf8 validation. Since internally the parser already works on byte strings, this should be possible and I personally already needed this in the past too.

~~I used the proposed approach from the issue by implementing `TryFrom<&'a [u8]>` for all 6 address types (3 ip address types and 3 socket address types). I believe implementing stable traits for stable types is insta-stable so this will probably need an FCP?~~

Switched to an unstable inherent method approach called `parse_ascii` as requested.

cc ```@jyn514```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 28, 2022
… r=joshtriplett

Support parsing IP addresses from a byte string

Fixes rust-lang#94821

The goal is to be able to parse addresses from a byte string without requiring to do any utf8 validation. Since internally the parser already works on byte strings, this should be possible and I personally already needed this in the past too.

~~I used the proposed approach from the issue by implementing `TryFrom<&'a [u8]>` for all 6 address types (3 ip address types and 3 socket address types). I believe implementing stable traits for stable types is insta-stable so this will probably need an FCP?~~

Switched to an unstable inherent method approach called `parse_ascii` as requested.

cc ````@jyn514````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 28, 2022
… r=joshtriplett

Support parsing IP addresses from a byte string

Fixes rust-lang#94821

The goal is to be able to parse addresses from a byte string without requiring to do any utf8 validation. Since internally the parser already works on byte strings, this should be possible and I personally already needed this in the past too.

~~I used the proposed approach from the issue by implementing `TryFrom<&'a [u8]>` for all 6 address types (3 ip address types and 3 socket address types). I believe implementing stable traits for stable types is insta-stable so this will probably need an FCP?~~

Switched to an unstable inherent method approach called `parse_ascii` as requested.

cc `````@jyn514`````
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2022
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#94890 (Support parsing IP addresses from a byte string)
 - rust-lang#96334 (socket `set_mark` addition.)
 - rust-lang#99027 (Replace `Body::basic_blocks()` with field access)
 - rust-lang#100437 (Improve const mismatch `FulfillmentError`)
 - rust-lang#100843 (Migrate part of rustc_infer to session diagnostic)
 - rust-lang#100897 (extra sanity check against consts pointing to mutable memory)
 - rust-lang#100959 (translations: rename warn_ to warning)
 - rust-lang#101111 (Use the declaration's SourceInfo for FnEntry retags, not the outermost)
 - rust-lang#101116 ([rustdoc] Remove Attrs type alias)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 52016a1 into rust-lang:master Aug 29, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 29, 2022
@marmeladema marmeladema deleted the ip-addr-try-from-bytes branch August 29, 2022 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a way to parse IP addresses without having to utf8-validate
10 participants