benoitc / erlang-idna

Erlang IDNA lib
MIT License
43 stars 29 forks source link

Update to support IDNA 13 #36

Closed ruudk closed 3 years ago

ruudk commented 3 years ago

It seems that Travis it not automatically building this PR. How to know if it's Ok?

ruudk commented 3 years ago

Added a test for the emoji conversion but it still fails, I don't understand:


  1) compat_test:emoji_test/0
     Failure/Error: {exit,
                        {bad_label,
                            {context,
                                "Codepoint 128065 not allowed ('DISALLOWED') at position 0 in [128065,128068,\n                                                              128065]"}},
                        [{idna,check_context,4,
                             [{file,
                                  "/Users/Ruud/Downloads/erlang-idna/src/idna.erl"},
                              {line,209}]},
                         {idna,check_label,4,
                             [{file,
                                  "/Users/Ruud/Downloads/erlang-idna/src/idna.erl"},
                              {line,254}]},
                         {idna,alabel,1,
                             [{file,
                                  "/Users/Ruud/Downloads/erlang-idna/src/idna.erl"},
                              {line,283}]},
                         {idna,encode_1,2,
                             [{file,
                                  "/Users/Ruud/Downloads/erlang-idna/src/idna.erl"},
                              {line,145}]},
                         {compat_test,'-emoji_test/0-fun-0-',0,
                             [{file,
                                  "/Users/Ruud/Downloads/erlang-idna/test/compat_test.erl"},
                              {line,22}]},
                         {eunit_test,'-mf_wrapper/2-fun-0-',2,
                             [{file,"eunit_test.erl"},{line,273}]},
                         {eunit_test,run_testfun,1,
                             [{file,"eunit_test.erl"},{line,71}]},
                         {eunit_proc,run_test,1,
                             [{file,"eunit_proc.erl"},{line,522}]}]}
     Output:
benoitc commented 3 years ago

Thanks for the PR!

After checking it, i believe the test doesn't pass because it's not valid for IDNA2008: https://util.unicode.org/UnicodeJsps/idna.jsp?a=👁👄👁.fm

or: https://util.unicode.org/UnicodeJsps/character.jsp?a=👁&B1=Show

not what to do, after all the lib is about validating against IDNA2003 and IDNA2008 :)

Maybe adding another option would work but unsure if we should do it. I would like to merge your changes for the version 13. Can you split your PR in 2 so we can handle the emoji issue separately?

ruudk commented 3 years ago

Just found out the same thing. TIL!

Updated the PR :)

benoitc commented 3 years ago

merged in latest master. Thanks!