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

Implementation of Email Address International support (RFC 8398) in OpenSSL #9654

Closed
wants to merge 10 commits into from

Conversation

beldmit
Copy link
Member

@beldmit beldmit commented Aug 21, 2019

RFC 8398 implementation in OpenSSL.

PUNYCODE decode is implemented from scratch.

Replaces #9199.

Checklist
  • documentation is added or updated
  • tests are added or updated

@beldmit beldmit requested a review from vdukhovni August 21, 2019 15:47
@beldmit
Copy link
Member Author

beldmit commented Aug 26, 2019

Ping @openssl/committers?

@slontis
Copy link
Member

slontis commented Aug 26, 2019

Is there any existing test data out there in the wild? (The spec seems to be somewhat lacking in this department). It would be good to interop with something.

@beldmit
Copy link
Member Author

beldmit commented Aug 26, 2019

Is there any existing test data out there in the wild? (The spec seems to be somewhat lacking in this department). It would be good to interop with something.

AFAIK, no external test data. I have a set of certificates used for test purposes and plan to provide them via test scenario.

@beldmit beldmit changed the title WIP: implementation of RFC 8398 in OpenSSL Implementation of RFC 8398 in OpenSSL Sep 18, 2019
@beldmit
Copy link
Member Author

beldmit commented Sep 18, 2019

Hopefully out of WIP.
Ping @openssl/committers

crypto/punycode.c Show resolved Hide resolved
@beldmit
Copy link
Member Author

beldmit commented Sep 25, 2019

ping @openssl/committers after rebase.
@vdukhovni, could you please take a look?

@beldmit
Copy link
Member Author

beldmit commented Sep 29, 2019

Force-pushed after #9333

@beldmit
Copy link
Member Author

beldmit commented Oct 7, 2019

Ping @openssl/omc

Copy link
Member

@levitte levitte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test files got misplaced

25-test_eai_data.t Outdated Show resolved Hide resolved
25-test_eai_data/ascii_chain.pem Outdated Show resolved Hide resolved
25-test_eai_data/ascii_leaf.pem Outdated Show resolved Hide resolved
25-test_eai_data/san.ascii Outdated Show resolved Hide resolved
25-test_eai_data/san.utf8 Outdated Show resolved Hide resolved
25-test_eai_data/utf8_chain.pem Outdated Show resolved Hide resolved
25-test_eai_data/utf8_leaf.pem Outdated Show resolved Hide resolved
crypto/build.info Outdated Show resolved Hide resolved
crypto/punycode.c Outdated Show resolved Hide resolved
crypto/punycode.c Outdated Show resolved Hide resolved
crypto/punycode.c Outdated Show resolved Hide resolved
25-test_eai_data.t Outdated Show resolved Hide resolved
include/internal/punycode.h Outdated Show resolved Hide resolved
@beldmit beldmit force-pushed the rfc8398_2ndtry branch 5 times, most recently from 313204c to 7de6407 Compare October 9, 2019 08:02
@beldmit
Copy link
Member Author

beldmit commented Oct 9, 2019

@levitte I hopefully implemented all the changes you've requested and made some git white magic to make the commits be logically consistent.

@levitte levitte self-assigned this Oct 9, 2019
Copy link

@weihaw weihaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies. This is my first time doing a github review let alone a openssl one. Please let me know if I'm supposed to submit comments differently.

crypto/punycode.c Outdated Show resolved Hide resolved
crypto/punycode.c Outdated Show resolved Hide resolved
crypto/punycode.c Outdated Show resolved Hide resolved
crypto/x509/v3_ncons.c Show resolved Hide resolved
crypto/punycode.c Outdated Show resolved Hide resolved
@bernd-edlinger
Copy link
Member

This is my first time doing a github review let alone a openssl one.

Thanks @weihaw, you're welcome here!

crypto/punycode.c Outdated Show resolved Hide resolved
crypto/punycode.c Outdated Show resolved Hide resolved
crypto/punycode.c Outdated Show resolved Hide resolved
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@openssl-machine openssl-machine added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Aug 26, 2020
openssl-machine pushed a commit that referenced this pull request Aug 26, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #9654)
openssl-machine pushed a commit that referenced this pull request Aug 26, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #9654)
openssl-machine pushed a commit that referenced this pull request Aug 26, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #9654)
openssl-machine pushed a commit that referenced this pull request Aug 26, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #9654)
openssl-machine pushed a commit that referenced this pull request Aug 26, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #9654)
openssl-machine pushed a commit that referenced this pull request Aug 26, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #9654)
openssl-machine pushed a commit that referenced this pull request Aug 26, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #9654)
openssl-machine pushed a commit that referenced this pull request Aug 26, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #9654)
@beldmit
Copy link
Member Author

beldmit commented Aug 26, 2020

Squashed and merged. Many thanks!

@beldmit beldmit closed this Aug 26, 2020
@beldmit beldmit deleted the rfc8398_2ndtry branch August 29, 2020 16:39
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9654)
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9654)
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9654)
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9654)
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9654)
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9654)
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9654)
swenkeratmicrosoft pushed a commit to swenkeratmicrosoft/openssl that referenced this pull request Sep 1, 2020
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from openssl#9654)
@henmaster
Copy link

This didn't age well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants